Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add object crosshair and disable entity selectionboxes by default #9523

Merged
merged 19 commits into from
Jul 14, 2020

Conversation

LoneWolfHT
Copy link
Contributor

@LoneWolfHT LoneWolfHT commented Mar 18, 2020

Goal of the PR

Add an indicator on the HUD to indicate when you are pointing at an entity. Similar to Minecraft's:
image

How does the PR work?

Replaces the default crosshair with a X when you are pointing at an entity
Comparison between HUD when pointing/not pointing at an entity:
image

The object crosshair is pretty much exactly like the regular crosshair in customization, it also uses the crosshair HUD flag to determine appearance

  • To define a custom texture for it use object_crosshair.png
  • To define a custom color use the object_crosshair_color setting
  • To define a custom alpha use the object_crosshair_alpha setting

This also disables entity selectionboxes by default

Does it resolve any reported issue?

Not a reported one, but @tacotexmex has been wanting entity selectionboxes removed for a while now.
Entity selectionboxes break immersion and are better suited for dev work IMO.
This PR will disable them by default without preventing players from knowing when they're pointing at an entity.

To do

This PR is Ready for Review.

How to test

  • Join the game and put your crosshair over an entity while within range of it (dropped items work well)
  • Make sure the object_crosshair settings (color/alpha) work
  • Make sure the crosshair/object_crosshair textures work and work together
  • Make sure the object crosshair doesn't appear when the HUD flag for the crosshair is disabled

I have tested all of the above without finding anything wrong

@paramat
Copy link
Contributor

paramat commented Mar 18, 2020

Nice idea.
Note: Changes to minetest.conf.example are not necessary and are wasted work, as that file is regularly autogenerated from settingtypes.txt.

@LoneWolfHT
Copy link
Contributor Author

Ok thanks. I'll remove that...

@tacotexmex
Copy link
Contributor

Nice improvement! 🎉

Is the position of the texture off-center? I’d suggest that it be completely centered and then offsetted in the texture itself. This way makers can choose any position of the indicator by customizing the texture.

@LoneWolfHT
Copy link
Contributor Author

Changed

@rubenwardy
Copy link
Member

I like this, but I'm concerned about how obvious this would be to the user. The sword like thing in MC looks like an entity health bar?

@LoneWolfHT
Copy link
Contributor Author

LoneWolfHT commented Mar 19, 2020

The sword is actually on the HUD most of the time, it displays the charge of the wielded tool. The plus icon below it only shows up when pointing at an entity. I guess I could have made it more clear
I think most players will be able to figure out what is going on pretty quickly. Since they'll only notice it when pointing at an entity

@tacotexmex
Copy link
Contributor

Come to think of it, why not define this as an alternative crosshair feature altogether? Meaning, not add this indicator on top of the existing crosshair but instead replace it?

@LoneWolfHT
Copy link
Contributor Author

Come to think of it, why not define this as an alternative crosshair feature altogether? Meaning, not add this indicator on top of the existing crosshair but instead replace it?

Replace it with what? Most of the time you should be able to just cover it up, the selectionindicator is drawn after the crosshair is

@paramat
Copy link
Contributor

paramat commented Mar 19, 2020

Meaning, not add this indicator on top of the existing crosshair but instead replace it?

Do you mean: the crosshair changes shape when pointing at an entity?

@LoneWolfHT
Copy link
Contributor Author

A setting for replacement could be added I guess

@sorcerykid
Copy link
Contributor

I really like the concept :)

But I have a dumb question: How is a selection indicator any less immersion breaking? I'm inclined to think it would be just as intuitive if not moreso if the crosshair simply changed shape to indicate that it's pointing to an entity or player. Having a dot showing the location of the entity almost seems like it would be more distracting than the selection box, esp. if the entity is moving.

Either way, it's still nice change of pace, and I welcome anything that helps to improve the gameplay experience.

@LoneWolfHT
Copy link
Contributor Author

LoneWolfHT commented Mar 20, 2020

Having a dot showing the location of the entity almost seems like it would be more distracting than the selection box, esp. if the entity is moving.

The dot doesn't show the location of anything, it's just a dot above the crosshair that appears in the same place every time when the player can 'interact with'/'reach' an entity

@sorcerykid
Copy link
Contributor

Ah, thanks for the clarification, I was under the impression it was centered on the entity. If it's simply being drawn above the crosshair, then I'm curious why not simply change the crosshair image itself?

@LoneWolfHT
Copy link
Contributor Author

Ah, thanks for the clarification, I was under the impression it was centered on the entity. If it's simply being drawn above the crosshair, then I'm curious why not simply change the crosshair image itself?

What would I change it to? Note: The default isn't an image, it's 2 lines drawn on the screen to form a '+'

src/client/game.cpp Outdated Show resolved Hide resolved
@tacotexmex
Copy link
Contributor

tacotexmex commented Mar 20, 2020

Do you mean: the crosshair changes shape when pointing at an entity?

Yes. Regular crosshair would use crosshair.png while the entity crosshair would use crosshair_entity.png or similar. The default look for the indicator can still be the crosshair combined with the dot, but this way it's more versatile imo.

@LoneWolfHT
Copy link
Contributor Author

The selectionindicator texture now replaces the crosshair.
Really need to think up a better name for the selectionindicator...

@paramat
Copy link
Contributor

paramat commented Mar 21, 2020

I have a suggestion for the form of the default selection indicator: Instead of a small solid-shaded square, use a small empty box formed by the same lines used to draw the crosshair. So 4 uses of driver->draw2DLine().
This will have more design consistency, be less obstructive to vision because it is not solid-shaded, and because it looks like a box it works like an icon that suggests pointing at the entity's selectionbox.

@LoneWolfHT
Copy link
Contributor Author

I'm not sure it would indicate a selectionbox very well but I'm up for that once someone else comments, the box does seem a little intrusive

@sorcerykid
Copy link
Contributor

sorcerykid commented Mar 21, 2020

I was actually going to suggest the exact same thing as paramat, but a diamond shape instead , just to make it even more distinctive. Although a square is perfectly fine by me as well. It's cool that I'm not the only one that was thinking along those same lines (no pun intended :)

@tacotexmex
Copy link
Contributor

As long as the look is fully customizable I don’t care about the default look.

@LoneWolfHT
Copy link
Contributor Author

I like the diamond idea, I think I'll do that tonight if nobody has any objections

src/client/game.cpp Outdated Show resolved Hide resolved
m_displaycenter.Y - (size.Y / 2));
driver->draw2DImage(selectionindicator, lsize,
core::rect<s32>(0, 0, size.X, size.Y),
0, selectionindicator_argb, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
0, selectionindicator_argb, true);
nullptr, selectionindicator_argb, true);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part is code I copied. Changed both the 'copied' and 'copied from' code

src/client/render/core.cpp Outdated Show resolved Hide resolved
@LoneWolfHT
Copy link
Contributor Author

Also changed indicator to a diamond
image

@LoneWolfHT
Copy link
Contributor Author

Made all but one of the requested changes and also rebased this so I could run a git diff to make sure I didn't miss anything

@SmallJoker
Copy link
Member

SmallJoker commented Jul 11, 2020

@LoneWolfHT Thank you for the changes. I'll re-review/retest soon.
Pro tip to avoid most of the rebase hell in the future:
git rebase -i HEAD~20 to squash most (or all) commits prior rebasing -> results in less rebase work and less errors.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested.

@SmallJoker SmallJoker merged commit 94619d8 into minetest:master Jul 14, 2020
@LoneWolfHT LoneWolfHT deleted the selection-indicator branch July 14, 2020 17:13
SmallJoker pushed a commit that referenced this pull request Jul 14, 2020
@paramat
Copy link
Contributor

paramat commented Jul 15, 2020

The selectionbox that appears around entities when you select them will be DISABLED BY DEFAULT in favour of the object crosshair

This is fine for me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.