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

Canonical mod name #67

Open
raymoo opened this issue Aug 5, 2016 · 11 comments
Open

Canonical mod name #67

raymoo opened this issue Aug 5, 2016 · 11 comments
Labels

Comments

@raymoo
Copy link

raymoo commented Aug 5, 2016

3d_armor uses a global table (armor) which differs from the mod name, and mod conventions are not to create any globals other than a single global table with the name of your mod.

Since Lua variables can't start with a digit, fixing this would mean changing the mod name to "armor", or changing both the mod name and table name to something else.

@stujones11
Copy link
Owner

I really don't see this as a problem unless some other mod were to use a global table named armor.

@raymoo
Copy link
Author

raymoo commented Aug 6, 2016

The problem as I see it is that someone else could make a mod called "armor", and then there would be no solution anymore but to change the table name, which would cause more breakage, since it is easier to change a line in depends than every reference to the table.

@stujones11
Copy link
Owner

There is already a mod called armor, by cornernote iirc, obviously it is not compatible with this mod.
I do understand what you are getting at, I just don't think it's worth changing now.

@BrunoMine
Copy link
Contributor

I believe the name should really be changed. Mainly because of grobal table. It is also a good code organization.

@stujones11
Copy link
Owner

The only thing I could possibly do is change the name of the main mod to just armor, however, that name is already taken and duplicate mod names are discouraged iirc. Not only that, any other mod that depends on 3d_armor will be broken and there are a few that are unlikely to ever be updated.

@BrunoMine
Copy link
Contributor

You're right, it really bad.

@rubenwardy
Copy link
Contributor

You should rename armor to 3d_armor, then deprecate armor (but keep an alias for backwards compat, by doing armor = 3d_armor)

@raymoo
Copy link
Author

raymoo commented Feb 16, 2017

@rubenwardy I don't think variables can start with a number in Lua. The mod name would have to be changed in addition to the table name (in which case the mod might as well be renamed armor)

@numberZero
Copy link
Contributor

@rubenwardy we don’t want to write _G['3d_armor'] in place of armor. Mod name change is not very good too, although as this is a modpack, that should not be a great problem (it might include a stub mod named 3d_armor)—but is that really necessary?

@raymoo
Copy link
Author

raymoo commented Feb 16, 2017

It's not necessary, neither is anything about code style or hygeine.

@stujones11
Copy link
Owner

stujones11 commented Mar 8, 2017

but is that really necessary?

If another mod depends on '3d_armor' (which quite a few do) then it won't find it if I rename it to 'armor'. A stub mod might just work but that would defeat the whole object, wouldn't it? As would aliasing the namespace, these could never safely be removed as many of these depending mods are never likely to be updated.

Perhaps if I can compile a list of all depending release mods and at least see how many are still maintained, I may consider removal of the '3d' prefix from the main mod name, although that would still conflict with the original 'armor' mod by cornernote (iirc) I have no idea if that mod is still in release or is even usable with the current game.

Trust me, I would love to rip this whole thing up and start again but that would be disrespectful to all those who have contributed, hosted and use this in sub-games.

AntumDeluge pushed a commit to AntumMT/mp-3d_armor that referenced this issue Sep 5, 2022
* add fire protection to nether armor
* add fire protection to nether shield
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants