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

Odd Essentials modifications #45

Closed
DorCoMaNdO opened this issue Mar 31, 2021 · 6 comments
Closed

Odd Essentials modifications #45

DorCoMaNdO opened this issue Mar 31, 2021 · 6 comments

Comments

@DorCoMaNdO
Copy link

The reasoning behind most of the modifications seems... odd? All they seem to do is break compatibility with other mods, why?
The whole point of Essentials is to encourage mods to be compatible with each other, yet the setting identifier no longer include plugin ids so plugins with the same setting id would conflict, the lobby text patch was also removed, which likely means you're patching the same method, but only displaying your own options, meaning other plugins adding options would result said options to not display.

Essentials is being constantly updated, you could always open an issue for a feature request or PR one if something is missing. I find compatibility breaking "specialized" forks to be silly, it just undermines what Essentials is meant to be, if you're not supporting plugins other than your own, your fork is not "Essentials".

@Eisbison
Copy link
Contributor

Eisbison commented Apr 1, 2021

I totally get your point and I'm on your side with that. I had the issue with people being unable to join lobbies because sharing the settings somehow didn't want to work when using string ids. Sadly I never got to the bottom of it, you probably did in the mean time? As my changes were not practical and as you're saying yourself... odd... there was no sense in opening a PR. You're right I should have opened an issue or feature request, sorry for that. You're also right with the lobby text patch, I just entered some dirty spacings/some tabs and colors in there (some of the features you already added to Essentials). I changed some accessibility levels because I needed access in order to add some settings presets you can switch through (probably not something that would make sense to be added to Essentials, especially not in the way that I've implemented it).

If possible I'd also be in my interest to switch back to the orginal Essentials (or maybe to a small fork where I just change the accessibility levels, as that would not break compatibility and enable me to implement my presets). I'll try to do that asap 👍 Is the issue still there regarding the string ids or am I missing something stupid there? :)

@DorCoMaNdO
Copy link
Author

The issue is regarding UDP packet sizes, 0.2.0-dev.8 will address this within a few days.

@Eisbison
Copy link
Contributor

Eisbison commented Apr 1, 2021

Awesome, then I'll switch back with the new version ✔️ Thank you for your work.

@DorCoMaNdO
Copy link
Author

0.2.0-dev.8 is now up, please let me know if you still experience a similar issue with the options not syncing, or any other issue you come across or feature you may need.

@Eisbison
Copy link
Contributor

Eisbison commented Apr 3, 2021

Thank you for the update, already switched locally 👍 I'm planning to still use a small variation of your Essentials, which only modifies 3 variablie accessibility levels (3x the ConfigEntry to pulbic). My use case is that I change the bind depending on a "preset option" (only for the settings that match my pluginId), so that I can store e.g. 4 different values for one option and switch between them by switching the preset setting. Do you still want me to create and link a fork there (as it doesn't affect compatibility at all and only three accessibilty levels changed) or are you fine with me just using that slight variation without pointing it out?

@DorCoMaNdO
Copy link
Author

DorCoMaNdO commented Apr 3, 2021

You can create derivatives of the option classes (in your mod) and add additional ConfigEntry fields for the presets, so you'd have one (the default) for the "active" preset and your additional ones to store presets, call SetValue to apply the selected preset.
And yes I'd like all modifications of Essentials to follow the license and post modifications of the code if any are made.

@Eisbison Eisbison closed this as completed May 3, 2021
gendelo3 pushed a commit to gendelo3/TheOtherRolesInofficial that referenced this issue Mar 5, 2022
JustASysAdmin referenced this issue in JustASysAdmin/TheOtherRoles2 Apr 29, 2022
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

No branches or pull requests

2 participants