Closed Twisol closed 11 years ago
Overall, it looks fine to me. I prefer that Michael looks over this too before it gets merged into master, though, since you did go into his menu system and change stuff around.
I think it's fine, it will probably not be noticeable as to having to change a lot of the screen classes.
I'm actually making a few more changes now. ^_^; I'm reworking how CreditsMenuScreen renders the list of credits, introducing a new IMenuEntry interface so that different kinds of menu entries can be rendered by MenuScreen.
Ok
Sent from my iPad
On Apr 10, 2013, at 4:04 PM, Jonathan Castello notifications@github.com wrote:
I'm actually making a few more changes now. ^_^; I'm reworking how CreditsMenuScreen renders the list of credits, introducing a new IMenuEntry interface so that different kinds of menu entries can be rendered by MenuScreen.
— Reply to this email directly or view it on GitHub.
To summarize this latest set of changes, I've decomposed the CreditsMenuScreen into a few smaller classes. There is now a LinesTemplate to which you can supply a series of IMenuLine items. IMenuLine currently only has two implementing types: Spacer and TextLine. I'm going to try using LinesTemplate in many of the other menus as well, since I saw a lot of repetitive position.Y += TextFont.LineSpacing
in a number of places. That means there should be a class MenuButton : IMenuLine
soon as well.
Also, yes, there's no documentation for the new code yet. I wanted to get the refactoring done first, so that the code might be semi-stable when I start writing it.
I'd appreciate another look at this set of commits before I do the merge.
Minor bug: in the Credits screen, the "Back" option isn't initially highlighted, and there seems to be an invisible menu entry, as you can highlight Back and then not highlight it again. Pressing Confirm (Enter or Space) takes the user back to the main menu regardless, but it does seem a bit off. In master, it works properly; the Back option is highlighted, and it's impossible to un-highlight it.
Ah, right. I did notice that; it's a side effect of how the MenuScreen machinery works. I plan on going back and making it possible to have non-selectable items. Those will probably be the MenuButtons I mentioned before.
In the interim, I made it so that the credits themselves (since they're selected first) will go back to the main menu as well. A bit funky, but it works.
Okay. I don't see anything else really, so just fix that and it should be fine...
Well, it's going to take a while, because that will affect the entire menu system. I'd like to push these changes in now and work through it as a second pull request. Does that sound okay?
That's fine; there's already enough changes in this pull request, and I know that you'll get the rest done :)
Thanks. Merged!
@Jjp137: Fixed the Credits screen back button issue in 3e52bf. Some of the code in LinesTemplate is terribad right now, but my changes are there in the menu-architecture
branch if you want to see my progress.
I'm drawing inspiration from the slide templates you get with Powerpoint or a Google Docs slideshow. So far, every menu fits one of two menu styles: the list of options, and the text blurb. I've factored these out so the menu screen classes themselves only deal with the data and the binding of actions to the buttons, allowing the general templates to handle rendering.
This is the first chunk of my current cleanups. The big change here is the refactoring of ScreenManager into a Scene class and a ScreenRenderer class, as well as a great deal of decoupling between the Screens and the ScreenRenderer. There's also a host of code style changes, mostly regarding auto-properties and code folding regions.
Looking forward, I want to frame the screen rendering subsystem as drawing a Scene composed of multiple Layers. Currently, the GameScreen and MenuScreen classes have a few too many responsibilities each, and I want to reduce the amount of inheritance-based coupling.