APercy / airutils

Airport Utils for Minetest
Other
4 stars 9 forks source link

Prefer biofuel mod biofuel instead of bundled biofuel? #5

Open Montandalar opened 10 months ago

Montandalar commented 10 months ago

I noticed you moved to bundle biofuel. I have not looked at 100% of the details of the implementation but I notice that it does support Mineclone2 unlike the original. Adding Mineclone2 support is good, but adding a separate mod seems like scope creep. I know it's the easiest way to update content from another mod instead of requiring a fork though. I also understand ContentDB doesn't really easily let you move users to a fork of an external mod; you would have to change the name of the biofuel mod.

Do you plan to keep compatibility with upstream biofuel (Lokrates version)? If so, I would try to use that version where it is installed. Check for minetest.get_modpath("biofuel"). Average Mineclone2 players will probably not have the biofuel mod; more advanced ones might use a designated fork.

In any case, what I would definitely not recommend is the current situation where for Minetest Game users, if they have the biofuel mod installed and enabled, they will still get the overrides applied. The idea should be (I though) to provide support where biofuel is already lacking, not to replace the separate mod.

So I would suggest some alternatives:

  1. Change the name of biofuel to biofuel_apercy or similar. Manage that mod separately. Prefer biofuel_apercy over biofuel and remove the bundled biofuel. ContentDB users will install biofuel_apercy by dependency resolution.
  2. Only run the bundled biofuel when running Mineclone2 or when Minetest Game users don't have biofuel. ContentDB users should get a suggestion to use biofuel; many existing users might already have it, but some new users may just use the bundled biofuel as biofuel is optional. Manual mod download users will probably fetch the separate biofuel themselves.

Anyway these are just some suggestions. I think it is nicer to keep the mod architecture clean. Thanks for your work on this library and your vehicles.

APercy commented 10 months ago

The solution is being tested yet. The support for Lokrates version will continue, but I'll add a method to add custom fuel too, for server owners. I'll for example make energy cells from basic machines usable on my server, using the new method. My internal biofuel mod needs some adjustments but I intend to keep it as an ace up my sleeve for other games, but with less options as Lokrates mod have. In other hand, I have an internal option to disable it (the now internal biofuel), but I'll let the server owner to decide, like they did on Your Land, disabling PAPI and thug tool.

APercy commented 10 months ago

changed my mind. If the server owner wants to force it enabled, just put this on minetest.conf: airutils.force_enable_biofuel = true It will be automatically enabled if it didnot find Lokrates biofuel

Lazerbeak12345 commented 3 months ago

You could do what farming redo does. Retain the same name, but have an indicator that it's enhanced with new capabilities.