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

Small cleanup of hud add/remove code #1332

Merged
merged 1 commit into from May 31, 2014

Conversation

Projects
None yet
2 participants
@sapier
Copy link
Contributor

commented May 25, 2014

No description provided.

@ShadowNinja

View changes

src/player.h Outdated
HudElement* removeHud(u32 id);
void clearHud();
u32 maxHudId()
{ return hud.size(); }

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja May 25, 2014

Member

This isn't attached to maxHudId() well. Use

u32 maxHudId() { return hud.size(); }

or

u32 maxHudId() {
    return hud.size();
}

or

u32 maxHudId()
{
    return hud.size();
}

This comment has been minimized.

Copy link
@sapier

sapier May 25, 2014

Author Contributor

Isn't according to our coding style only the last one acceptable? ;-)

return false;

delete player->hud[id];
player->hud[id] = NULL;
HudElement* todel = player->removeHud(id);

This comment has been minimized.

Copy link
@ShadowNinja

ShadowNinja May 25, 2014

Member

Rather than having to delete the returned pointer, I think it would be better if removeHud() deleted it if possible and returned a boolean indicating success.

This comment has been minimized.

Copy link
@sapier

sapier May 25, 2014

Author Contributor

I did this to get symmetry, the one allocating the hud element should be responsible for deleting it too. The hud list doesn't create it so it's not supposed to delete it.

@sapier sapier merged commit d76b8c6 into minetest:master May 31, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details

@sapier sapier deleted the sapier:small_hud_cleanup branch Jan 10, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.