-
Notifications
You must be signed in to change notification settings - Fork 188
Move the gui windows to comptime modding #1630
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
Move the gui windows to comptime modding #1630
Conversation
|
I am not sure if this is best choice, we might consider API changes which would allow us to spawn multiple instances of same window, unless that was rejected (?) |
|
Also, please don't mention import user, instead use back ticks to make a code block |
That was not rejected, however I do not think we have to worry about it here. Just because we support modding, doesn't mean we need to commit to this as the final window API. |
| const Vec3d = vec.Vec3d; | ||
| const Mat4f = vec.Mat4f; | ||
| const gpu_performance_measuring = main.gui.windowlist.gpu_performance_measuring; | ||
| const gpu_performance_measuring = main.gui.windowlist.@"cubyz:gpu_performance_measuring"; |
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.
While reviewing this I noticed these ugly @"" as well as the fact that this import style is technically going against the guidelines, since you are renaming it. And I have an idea how we could fix this, and I would like to know what you and @Argmaster think about this:
What if we namespace mods?
So instead of
pub const @"cubyz:..." = @import("cubyz/rotation/...");
pub const @"cubyz:..." = @import("cubyz/rotation/...");we just keep the list and import it instead?
pub const cubyz = @import("cubyz/rotation/_list.zig");Then in cases like this we could just do main.gui.windowlist.cubyz.gpu_performance_measuring.
Furthermore this would allow us to keep the advantage of a manual list: We can organize files in subdirectories independently of the actual id, like it's done for worldedit commands.
The only downside is that it would make loading more difficult, but that shouldn't be a big deal, we could just add a iterator thing to e.g. assets.zig for this purpose.
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.
Yeah, should be fine.
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.
What if we keep the @ notation in the list struct for use from zon files but have a separate autogenerated struct that uses this syntax for use from the code itself?
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.
Hmm don't we need the "addon:path" notation to be able to properly map values (for eg. rotation option string value) from zone files to correct implementation?
If that's the case, what the codemob suggests is the only way forward the most straightforward way forward C:
We should still support the organization through folders inside each moddable thingy (rotations/windows etc.)
|
What is the status of this? |
Closes #1537
The main changes to the gui window code is that I made it use main.gui instead of
@import("../gui.zig"); and i also use GuiComponent.Button instead of@import("../components/Button.zig")and the same for the rest of the components. I also had to change the code that references gui windows to have a cubyz: prefix