-
Notifications
You must be signed in to change notification settings - Fork 31
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
Change mod ID regex to allow '.' in mod IDs #203
base: main
Are you sure you want to change the base?
Conversation
|
nope, cursed Edit:
Still cursed. - and _ are ok but my question would be, why have a . In the modid? Splitting words makes more sense with - or _. Not a . |
Imagine the case of subcomponents that are mods bundled in a parent mod. The parent is Furthermore, I have several libraries that are non-neo-specific and are distributed in several parts -- one that's, I dunno, a LIBRARY, one that's a GAMELIBRARY, and one that's a mod. When the first two are effectively , by module name, Finally, I'd say the question is instead: why shouldn't someone be able to use a dot in a mod ID? If they want to, there's no practical reason it shouldn't work -- they're valid in RL namespaces and module names both, after all. Maybe you want to organize your mod ID with underscores but that doesn't mean everyone has to when there's no actual reason not to allow dots seeing as MC does in namespaces. |
w.r.t. the breaking-change label: We're changing what our public API methods can return by implementing this change. We should make the conscious choice to relax the constraint. |
This does not change what any public API methods may return. getModId could have returned a mod ID with dots in it before if you were working with a custom IModInfo, which with custom mod file readers possibly present you may have been, since there is no documented contract specifying what implementations may or may not return there. That said, if there are mods out there (incorrectly) relying on this it's not terrible to delay it till 1.21.2 just in case -- I just wanted to be clear that there is no contract in FML, explicit or implied, that this breaks. Delaying it also means we can place such a contract on the method, which is breaking. |
That is a good point! (edit: we should still probably warn people that we're making this change, heh) |
I'd say let's leave it to 1.21.2 so that we can add such a restriction to the method -- I've updated the PR since you originally added the breaking changes label to document that. I feel like it's a good thing for modders to be able to rely on. |
Why are dashes ( |
We use ModFile == Java module semantics and Java modules do not allow dashes |
The first two |
No, they should be |
Aha, so there is |
I can't currently find a justification for why this isn't allowed -- dots are allowed in both RL namespaces and module names, and having a mod ID that is a proper module name is nice.
If this is not desired, we should document the reasoning behind the current regex and why dots aren't allowed.