Skip to content
This repository has been archived by the owner on Apr 22, 2021. It is now read-only.

ModuleManager fixes / improvements #294

Merged
merged 3 commits into from
Dec 17, 2019
Merged

ModuleManager fixes / improvements #294

merged 3 commits into from
Dec 17, 2019

Conversation

EmotionalLove
Copy link
Contributor

@EmotionalLove EmotionalLove commented Dec 17, 2019

custom module names aren't known as compile-time, so if a user renamed "Freecam" to "Spectator", or something similar, the lookup map would surely map "Spectator" to the "Freecam" module, but the code still asks ModuleManager for "Freecam" regardless.

@EmotionalLove EmotionalLove requested a review from 5HT2 December 17, 2019 15:52
@x4e
Copy link
Contributor

x4e commented Dec 17, 2019

Why would people be renaming module names? If they are then they should rename all references themselves. This creates unreadable code, where you see something requesting a module "Spectator", you look for the spectator module, but then you cant find the spectator module.

@EmotionalLove EmotionalLove merged commit 52af5bb into master Dec 17, 2019
@EmotionalLove EmotionalLove deleted the modulemanager branch December 17, 2019 15:59
@EmotionalLove
Copy link
Contributor Author

Why would people be renaming module names? If they are then they should rename all references themselves. This creates unreadable code, where you see something requesting a module "Spectator", you look for the spectator module, but then you cant find the spectator module.

again, don't talk to me about this. @zeroeightysix made the modules renameable iirc

@x4e
Copy link
Contributor

x4e commented Dec 17, 2019

Do you mean them being renamed in the code or is there some sort of runtime mechanism for updating their names?

@x4e
Copy link
Contributor

x4e commented Dec 17, 2019

If its the case that people can rename them at runtime you should instead make it update the name that is displayed in the gui rather than the internal name.

@5HT2
Copy link
Member

5HT2 commented Dec 17, 2019

they're updated in Module.Info, config side, but for enabling modules by name it looks for literally their name

@5HT2
Copy link
Member

5HT2 commented Dec 17, 2019

I'm pretty sure it doesn't change the internal name?

@5HT2
Copy link
Member

5HT2 commented Dec 17, 2019

It is updated gui side and cli side, otherwise it's the same in code

@EmotionalLove
Copy link
Contributor Author

Do you mean them being renamed in the code or is there some sort of runtime mechanism for updating their names?

renamed during runtime. you could rename the modules using a command. There's a field called originalName that keeps the original name, I updated ModuleManager to rely on that instead of the name field, which can change.

@x4e
Copy link
Contributor

x4e commented Dec 17, 2019

Why not create a method getDisplayName() which returns a user friendly name of the module for the arraylist and gui. Then users can update that user friendly name. This means that all references to the module will remain the same, stopping configs and string lookups from breaking.

@5HT2 5HT2 added this to the v1.1.2 milestone Dec 17, 2019
@kami-blue kami-blue locked as resolved and limited conversation to collaborators Dec 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants