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

Move the nametag back to the top of the player #6179

Merged
merged 1 commit into from Jul 29, 2017

Conversation

TeTpaAka
Copy link
Contributor

See #6174

@nerzhul
Copy link
Member

nerzhul commented Jul 27, 2017

it's not the correct fix, as now we can change model, it's not a fixed size like before. Can you find the correct value of the model instead ?

@TeTpaAka TeTpaAka force-pushed the master branch 2 times, most recently from 7d8b34e to 4fcce8b Compare July 27, 2017 09:10
@TeTpaAka
Copy link
Contributor Author

Updated.

src/camera.h Outdated
@@ -157,7 +160,8 @@ class Camera
}

Nametag *addNametag(scene::ISceneNode *parent_node,
std::string nametag_text, video::SColor nametag_color);
std::string nametag_text, video::SColor nametag_color,
Copy link
Member

Choose a reason for hiding this comment

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

nice, can you just fix to use const ref on the string and the v3f please ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

@nerzhul nerzhul left a comment

Choose a reason for hiding this comment

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

nice job

@@ -895,8 +895,11 @@ void GenericCAO::addToScene(ITextureSource *tsrc)
scene::ISceneNode *node = getSceneNode();
if (node && m_prop.nametag != "" && !m_is_local_player) {
// Add nametag
v3f pos;
pos.Y = m_prop.collisionbox.MaxEdge.Y + 0.1f;
Copy link
Contributor

@paramat paramat Jul 27, 2017

Choose a reason for hiding this comment

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

Probably needs to be higher, as before it was position + 1.1, which was 2.1 above foot position, the collisionbox top is at 1.77 above foot position, so add 0.3f for consistency and some clearence?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@paramat paramat added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Jul 28, 2017
read the actual height of the collisionbox
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Tested. Works.

@SmallJoker SmallJoker merged commit d504831 into minetest:master Jul 29, 2017
osjc pushed a commit to osjc/minetest that referenced this pull request Jan 11, 2019
read the actual height of the collisionbox
osjc pushed a commit to osjc/minetest that referenced this pull request Jan 23, 2019
read the actual height of the collisionbox
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Action / change needed Code still needs changes (PR) / more information requested (Issues) Bugfix 🐛 PRs that fix a bug >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants