pandorabox-io / pandorabox_custom

pandorabox server customization mod
https://pandorabox.io
MIT License
4 stars 7 forks source link

Refactor / split "pandorabox_custom" #60

Open dennisjenkins75 opened 1 year ago

dennisjenkins75 commented 1 year ago

This is a place-holder bug for discussion / coordination.

The server EdenLost also uses this mod. It provides many useful things, but is somewhat of a "kitchen sink". I would like to modify some of the bits, but need to do so in a way that is convenient for both Pandorabox and EdenLost. Specifically, I think that it would be beneficial to split the following "sub mods" out of this mod into their own projects (this list is not exhaustive), and then make the following changes (or I would then fork the new modules entirely, as needed). The tricky part is that removing code from pandorabox_custom is not atomic with updating the pandorabox_mods repro so that Pandorabox would never be without its mods.

I would prefer to not simply fork pandorabox_custom as EdenLost_custom and play complex partial merge games.

The bikeshed is green. Please advise. :)

BuckarooBanzay commented 1 year ago

The tricky part is that removing code from pandorabox_custom is not atomic with updating the pandorabox_mods repro so that Pandorabox would never be without its mods.

This isn't an issue, i can do this manually or whip up a PR that combines both mod changes

xp_priv.lua - I'm fine with the core code, but I'd like to add some privs for EdenLost that do not exist in Pandorabox. Maybe we can make the local privs table something that it loaded from disk at mod init time ("dofile ...")

the privs table is used dynamically and additional privs (or all of them 🤔) could be also added via a simple api

function xp_privs.register(xp, name)
 table.insert(privs, { xp = xp, name = name })
 -- sorting?
end

(i'm fine with just the exposing the variable too)

onplace_restrictions.lua I'd like to change it from having a hard-coded table of item -> XP mappings to reading this data from a config file, and using a priv check instead of an XP check. Further, I'd like to split the bucket:bucket_lava check into two: One priv for placing lava at Y<=-3 (granted at 10k XP), and a different priv for placing lava at y>3 (granted at 100k XP).

We can find a common ground too if you want to keep the checks simple, the separate privs for underground/above ground placing is fine too IMO

spawn_fast_walk.lua - I need to disable this, or make the local range configurable.

My suggestion: a setting to enable it (default is disabled) and a setting for the range EDIT: fixed in #61

aliases.lua - This needs to be made separate, as it prevents EdenLost from using some mods that are aliased away from.

62

chat/spawn.lua - I need to customize this to disable "/spawn" if the player holds a different privilege (jail).

👍

travel/travel.lua - I want to place a new layer above Y>=18000, but cannot due to this mod.

The max_height could be made configurable or i could add a simple registration api, as you want...

The bikeshed is green. Please advise. :)

I prefer yak-shaving :P

BuckarooBanzay commented 1 year ago

Another solution would be to introduce features according to server_name:

local server_name = minetest.settings:get("server_name")
if server_name == "Pandorabox" then
 -- something, something
elseif server_name == "Edenlost" then
 -- something, something else
end

Also: i added you as maintainer here, just in case 😉

dennisjenkins75 commented 1 year ago

Another solution would be to introduce features according to server_name:

IMHO, this is not a good idea. Its brittle, tightly couples the mods to our servers, and breaks mod behavior when I test things in a backup copy of the world (I set the name differently, b/c IRC bridge nick deduplication reasons. EdenLost uses Edgy's IRC framework, not beerchat; although beerchat is likely better; meh).

Maybe pandoabox_custom could just read sub-mod enablement flags from minetest.settings():get_bool() or something.

Although it is more work, ISTM that the cleanest solution is still to fragment pandorabox_custom, so that some submods could be forked (if needed), and others can be omitted entirely, on a per server basis.

Thank you for your responses. I will review them all in more detail after I have my :coffee: :)

local server_name = minetest.settings:get("server_name")
if server_name == "Pandorabox" then
 -- something, something
elseif server_name == "Edenlost" then
 -- something, something else
end

Also: i added you as maintainer here, just in case wink

Thank you. I shall use my powers responsibly.

birdlover32767 commented 5 months ago

well what about placing lava from -2 to 3, what xp does that need? 1 mil?