Main Menu Work

[jaredready]

Selected menu button now changes color when selected. Selection can happen from mouse moving over, or up/down key pressed. Enter (return) key 'presses' the currently selected button.

[jaredready]

Fixed.

[LB--]

I'm not too happy with the way keyboard navigation is implemented, it's not easily expandable as we add more options on the menu and it looks overly complicated as-is. Maybe it would be better if we had a std::vector<std::reference_wrapper<sf::Text>> and used an iterator to indicate which was selected - anything but the current if-else mess.

I'm also sick with a bathroom-related illness so I can't really do much right now, sorry if I seem irritable.

[jaredready]

Totally agreed. I had tried a couple different implementations, but they all ended up causing more trouble, without actually making anything easier. I hadn't thought of using std::reference_wrapper though. I'll give that a try. I have another idea, but it will require a bit of abstraction. It may good in the long run though.

Living off of chocolate-flavored instant breakfast probably isn't great for ones stomach :)

[jaredready]

Alright finished a good deal of work on abstracting this in such a way to make it easy to add more menu items. I don't think is ready for merging yet, but I'd like to some input.

I have some concerns with how this is currently implemented, and I believe I have these areas commented. Let me know what you think.

[LB--]

Do we really need ButtonManager? If the answer is still yes, it should be more abstracted - after all more than just buttons can benefit from the behavior of this class, since all the class does is hold a vector and track an iterator to it. But really having an entire class for a vector and an iterator seems a bit overkill.

[jaredready]

I think it could renamed to something along the lines of ButtonGroup. It does more than just hold a vector and iterator. It's basically modeled off of Java's ButtonGroup.

It gives us the ability to have mutually exclusive groups of buttons. I found this easier than not having the class at all.

[LB--]

It does more than just hold a vector and iterator.

You're right, it holds a vector and a pointer instead. My point is that it should be more abstract than just being limited to Buttons. What exactly does it do that only works for Buttons? Looking at it, it seems to be very concrete and does a lot of things with magic numbers, such as setting colors, etc.

It's a god idea but we need to go more abstract with it. I'm starting to recover from being sick so maybe I'll fork your branch and submit a PR to you, haha.

[jaredready]

I can pretty easily break it from its sfml ties but I don't think it should be abstracted to the point of working with more than just Buttons. IMO, not everything need be abstract. On Jan 9, 2014 8:24 PM, "LB--" notifications@github.com wrote:

It does more than just hold a vector and iterator.

You're right, it holds a vector and a pointer instead. My point is that it should be more abstract than just being limited to Buttons. What exactly does it do that only works for Buttons? Looking at it, it seems to be very concrete and does a lot of things with magic numbers, such as setting colors, etc.

It's a god idea but we need to go more abstract with it. I'm starting to recover from being sick so maybe I'll fork your branch and submit a PR to you, haha.

— Reply to this email directly or view it on GitHubhttps://github.com/cpluspluscom/ChessPlusPlus/pull/80#issuecomment-31997771 .

[LB--]

Oops, I forgot this existed when I made #87 - let me check this out

[LB--]

After fixing all the for loops (I have warnings treated as errors), I was able to successfully compile and test this out - it works.

I'm a but concerned about problems with how C++'s modulus % operator works - are you sure your code works in all cases?

[LB--]

I just noticed, it seems the original branch or repo for this pull request was deleted, so this PR can no longer be updated. I created the rb-menu branch for you to recover it. I think this PR is dead, though, and we'll have to create a new one.