-
Notifications
You must be signed in to change notification settings - Fork 21
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
(DRAFT) Add theme support #73
base: master
Are you sure you want to change the base?
Conversation
#include <windows.h> | ||
#endif | ||
|
||
void findThemes(std::string const& path, MenuThemes *list) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not as part of MenuThemes::
?
void MenuThemes::execute(Menu *menu) { | ||
if (menu->getCurrentMenu() != this) { | ||
menu->setCurrentMenu(this); | ||
} | ||
else { | ||
if (childNodes.size() > selected) { | ||
this->childNodes.at(selected)->execute(menu); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't do anything different from MenuNode
, does it?
In that case, this can be removed.
I'd wager you want ThemeItem
to perform the actual magic in the end anyways?
class MenuThemes : public MenuNode { | ||
public: | ||
MenuThemes(MenuNode *parent, std::string const& label, std::string const& path); | ||
~MenuThemes(); | ||
void execute(Menu *menu); | ||
protected: | ||
std::string path; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worthy of its own file, no?
I'd very much prefer 5 files (3 images, 1 font, one config-file in JSON format) over one big blob. |
+1 to JSON over a custom blob format that will require a tool to create. I believe you can count on the "A:" containing whatever is in the NevolutionX directory (see e.g. a
Then we can just have a convention that these will live in subdirectories under a "Skins" directory, e.g.: |
Man, been a while... I'll go with JSON then. Having some more experience now I see why it's better. I'll also get to fixing/improving based on the code reviews. On to work then... |
Hello!
Theme support has been listed in the README to-do list for a while, but has never been implemented. As an exercise, I decided to make this a reality myself.
I decided to start out with just the basic code for the menu item instead of the entire thing as I predict there will be lots of things to implement and just wanted to open a PR to let people know that this is "under construction".
Currently, here's what the code does:
It can now create a "theme" type menu (like scan and settings are different types) that once launched will look at the given path for .nevoxtheme files. Unfortunately this is all for now, however I've tried to figure out the .nevoxtheme file format.
At first, I thought it'd just be a special JSON file that has a few key/value pairs:
The name could be taken from the theme file name or from a new key/value pair.
However, I decided that due to complications with paths on Xbox like D: being automounted as the XBE path and the rest not being always mounted, we can do better.
This is what I came up with:
Another reason I thought of that format is ease of use. Using the first file format, that'd need us to copy 5 files to have a complete, working theme. With this format, we only need 1 big file.
A couple of disadvantages I've thought of for the 2nd format are:
I'm ready to hear your thoughts on both the code and the themes format. Cheers!