Closed wsor4035 closed 1 year ago
ethereal yet again is still nuking things so a dep on it is needed to prevent this.
isn't the "nuking" (clearing biomes/decorations and re-registering them) now a configurable behavior?
iirc its now a setting, but it should still properly clear and reregister decorations
The "nuking" is in "biome_init.lua" in ethereal's biome_ini.lua which is only triggered when ethereal.clear_default_biomes
is set true (which it is by default). I happened to be looking at it last night.
The logic seems to be
I'm quite new here and to this codebase; there are of-course probably reasons why ... however I don't quite get why minetest.unregister_biome
isn't used selectively on items in the discard list nor why decoration removal isn't the same. Also seems that the current method deprives all biomes of certain decos if they appear in even one discard-list biome. Feels like it's removing too much from the game at first glance.
I'd be happy to try to solve this in ethereal and submit a pull request there. Does anyone know off-hand one-or-more decos which are disappearing or should I just dump them and look for a moretrees:
prefix?
the simple way to test is clone both mods, remove the ethereal dep from moretrees, and add a moretrees dep to ethereal, and then see whats missing
ideally ethereal should keep its hands off anything that isnt made by it, and/or its directly modifying (like default)
Based on my tests, I cannot find any indication that Ethereal is removing moretrees decos. If I missed a case or you'd like something else validated, please let me know.
Test setup:
minetest.register_on_mods_loaded
to dump deco info after all mods are loaded, ethereal, and moretreesSampling procedure:
Cases sampled:
Moretrees adds a "decoration" field with a "moretrees:" prefix. This field is included in the extracted log info
General Conclusions:
Attached are the lists of decos found in each case in case someone wants to double check my analysis 'cause it's fairly late here. I've also attached a zip with the debug mod I wrote for testing this.
decos_case_4_moretrees_and_ethereal_deps_reversed.txt decos_ethereal_alone.txt decos_case_1_none.txt decos_case_2_moretrees.txt decos_case_3_moretrees_and_ethereal.txt awk_stats.zip
Edit: Missed the starting of the world in the sampling procedure.
Assuming I haven't missed anything, can we remove that dependency on ethereal from moretrees? Is there something else I should try out? I'm happy to reopen my pull request which effects this change if it's helpful.
+1 on just removing the dependency. what is to be done about ethereal is a separate discussion.
@Niklp09 since you made the original claim https://github.com/mt-mods/moretrees/pull/11#discussion_r1002973356 here, is this still a valid issue?
Looked into ethereal's biome_init.lua
's history a bit and commit ff99917e is very informative. The behavior @Niklp09 described is corrected in this commit; but anyone who has the previous version of ethereal and hasn't updated it would still fall pray. I can imagine this causing confusion long after the bug is fixed.
In the above commit, ethereal's version is changed to 20220424
; If we want moretrees users to avoid this confusion and think it's worth it, we could do a version check and dump a warning. I'm imagining something like:
if ethereal then
ethereal_ver = tonumber(ethereal.version)
if (ethereal_ver and ethereal_ver < 20220424) then
minetest.log("warning", "The version of ethereal detected can result in parts of this mod and others disappearing due to mod load order. Please update ethereal.");
end
end
I'm not sure it's worth it though as the first thing we can ask is what version of ethereal they have if someone reports the issue. Depends on the expected frequency that people will have an old version of ethereal and a new version of moretrees.
your code would need to be wrapped in a register on mods loaded to always work without a dep on ethereal
proposal:
unless @Niklp09 can show the situation is still broken
reopen https://github.com/mt-mods/moretrees/pull/21 - request a version of https://github.com/mt-mods/moretrees/issues/22#issuecomment-1345102761 snippet be added wrapped in register on mods loaded.
personal opinion on the snippet, iirc, that will only write to debug.txt which is more dev focused. instead or in addition to it should write something into the chat for the first player warning them, or similar to i3 does, block the server process from starting, since its not going to work properly anyways.
perhaps that the above would be better suited for a review in the pr if the above proposal is adopted
block the server process from starting, since its not going to work properly anyways.
this ^ would be the way imo (if we are going the hacky route)
granted no one has said anything here/discord/irc/matrix in 15h, just going to reopen the pr, and request changes
@wsor4035 all done in the pull request. Not sure how to mark this as resolved by that. Please let me know if more is needed.
ethereal yet again is still nuking things so a dep on it is needed to prevent this. known circular dep tree:
ethereal --> bakedclay --> flowerpot --> moretrees --> ethereal.
other issue trackers mentioning this: https://gitea.your-land.de/your-land/bugtracker/issues/3013 (thanks Niklp)status: need to talk to tenplus1