Skip to content
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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lukebemish
Copy link
Contributor

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.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@TelepathicGrunt
Copy link
Contributor

TelepathicGrunt commented Sep 15, 2024

the.bumblezone

nope, cursed

Edit:

repurposed.structures
repurposed-structures
repurposed_structures

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 .

@lukebemish
Copy link
Contributor Author

lukebemish commented Sep 16, 2024

Imagine the case of subcomponents that are mods bundled in a parent mod. The parent is foobar, and it bundles, say, foobar.psi_epsilon and foobar.alpha_delta as subcomponents.

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, dev.lukebemish.foobar.gizmo and dev.lukebemish.foobar.gadget, and the third is foobar_widget, it's weird.

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.

@shartte
Copy link
Contributor

shartte commented Sep 17, 2024

w.r.t. the breaking-change label: We're changing what our public API methods can return by implementing this change.
Some middleware might have relied on mod-ids being simple Java identifiers (i.e. implementing script-language access to mods via some global mods.<id> mechanism.

We should make the conscious choice to relax the constraint.

@lukebemish
Copy link
Contributor Author

lukebemish commented Sep 17, 2024

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.

@shartte
Copy link
Contributor

shartte commented Sep 17, 2024

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,

That is a good point!

(edit: we should still probably warn people that we're making this change, heh)

@lukebemish
Copy link
Contributor Author

lukebemish commented Sep 17, 2024

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.

@Fuzss
Copy link

Fuzss commented Sep 18, 2024

Why are dashes (-) not allowed? Asking as they are on Fabric.

@shartte
Copy link
Contributor

shartte commented Sep 18, 2024

We use ModFile == Java module semantics and Java modules do not allow dashes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants