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

Button hudget #546

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Button hudget #546

wants to merge 30 commits into from

Conversation

gersseba
Copy link
Contributor

Depends on #588

Buttons with std::function Callbacks, implementation example in hud.cpp

… button_hudget

Conflicts:
	src/ui/hud/hud.cpp
	src/ui/hud/textfieldhudgetvoxels.cpp
	src/ui/hud/textfieldhudgetvoxels.h
… button_hudget

Conflicts:
	src/ui/hud/hud.cpp
	src/ui/hud/hud.h
	src/world/world.cpp
	src/world/world.h
@gersseba
Copy link
Contributor Author

that was not supposed to be pushed, i'll fix it

for (int i = 0; i < width; i++) {
for (int j = 0; j < height; j++) {
if (i == 0 || j == 0 || i == width - 1 || j == height - 1) {
m_buttonVoxels->addVoxel(new Voxel(glm::ivec3(i, j, 0), 0x0FF00F));
Copy link
Member

Choose a reason for hiding this comment

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

I assume these are border and content colors? please make them constants

Copy link
Member

Choose a reason for hiding this comment

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

or even better: configurable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will declare them at the beginning of the method (as it is done in all other hudgets). I created a #554 which addresses the issue of a configurable color theme.

… button_hudget

Conflicts:
	src/ui/hud/hud.cpp
	src/ui/hud/hud.h
	src/ui/hud/textfieldhudgetvoxels.cpp
	src/ui/hud/textfieldhudgetvoxels.h
	src/world/world.h
TextFieldHudgetVoxels::draw();
}

void ButtonHudgetVoxels::setContent(std::string content) {
Copy link

Choose a reason for hiding this comment

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

constref ;)

@@ -258,4 +266,7 @@ void HUD::setView(const View* view) {
m_view = view;
}

void HUD::openMenu(ClickType clicktype) {
glow::debug("Not yet implemented");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

when should a callback with clicktype none be fired? Also, when I click I don't get debug output atm

@psieg
Copy link
Contributor

psieg commented Apr 13, 2014

@gersseba ping

@gersseba
Copy link
Contributor Author

Well creating an enum with only one instance seemed wrong, once someone comes up with a different clicktype none can be replaced.

@psieg
Copy link
Contributor

psieg commented Apr 17, 2014

I'd really like

  • if you have the clicktype enum, allow both, "select" and "fire" on the button (at least in a menu I want to left-click on buttons to use them, this could be done by changing the button meaning though...)
  • I think a button version with a border but without a background would be nice

:shipit: either way. Please consider making the button useful though, maybe reset the scenario or something...

@psieg
Copy link
Contributor

psieg commented Apr 21, 2014

this one needs merge, work, reviews and/or discussion...

@mrzzzrm
Copy link

mrzzzrm commented May 18, 2014

Crashes on multiple clicks on Reset

… button_hudget

Conflicts:
	src/ui/hud/hudelements.cpp
	src/ui/hud/textfieldhudgetvoxels.h
@gersseba
Copy link
Contributor Author

@mrzzzrm fixed the bug, if possible review again

@psieg
Copy link
Contributor

psieg commented Jul 31, 2014

the github diff is rather useless here atm.
If you want this merged for the clean state please make it mergable.
Afair there were no real objections to this feature and now that there isn't going to be the awesome menu-around-a-planet this will be it I guess :)

… button_hudget

Conflicts:
	src/display/rendering/starfield.h
@gersseba
Copy link
Contributor Author

gersseba commented Aug 1, 2014

Well awesome, the mergeconflict was an empty line note being empty enough...

@gersseba
Copy link
Contributor Author

gersseba commented Aug 4, 2014

@psieg done merging, can this be shipped then?

@psieg
Copy link
Contributor

psieg commented Aug 8, 2014

I'm ok, just have someone else confirm :P

@gersseba
Copy link
Contributor Author

gersseba commented Aug 8, 2014

fair enough, I'll just strap on my confirmation-helmet, squeeze into my confirmation-cannon and fire of into confirmation-land 😉

m_soundManager(new SoundManager())
{
World::instance()->player().hud().resetButton()->setCallback((std::function<void(ClickType clickType)>)std::bind(&GamePlay::resetButtonCallback, this, std::placeholders::_1));
Copy link

Choose a reason for hiding this comment

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

this line seems to be the same as setResetCallback(). Please use this method then.

@mrzzzrm
Copy link

mrzzzrm commented Aug 10, 2014

Still quite some comments from my side. I'm leaving again on Wednesday morning, so until then I will look into your fixes ;) Otherwise I'll return on the 17th.

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

Successfully merging this pull request may close these issues.

4 participants