diff options
author | Y. Wang <y5nw@protonmail.com> | 2024-09-19 18:39:19 +0000 |
---|---|---|
committer | orwell <orwell@bleipb.de> | 2024-11-09 22:09:00 +0100 |
commit | 66868e2eefe97b7a6d35fa3fecc895b9c7bbc4cc (patch) | |
tree | 5fdad135e06648875d9a050852c98e215192531d | |
parent | 86e80e4cfb0d7017ff962e40ab99a4ab6addf8f2 (diff) | |
download | advtrains-66868e2eefe97b7a6d35fa3fecc895b9c7bbc4cc.tar.gz advtrains-66868e2eefe97b7a6d35fa3fecc895b9c7bbc4cc.tar.bz2 advtrains-66868e2eefe97b7a6d35fa3fecc895b9c7bbc4cc.zip |
Address wagon aliasing issues
As it turns out, not fully testing new features is not necessarily a
good idea ...
This patch follows up 1F616EMO's patch by
* Making get_wagon_prototype return the resolved alias,
* Handling recursive wagon alises (in particular, loops), and
* Adding (partial) unittest for the wagon aliasing system. [v2]: The
testcases are complemented a bit more to cover situations where the
alias resolution system should return nil.
[v2]: This patch should hopefully also warn about not spawning wagons.
Note that this only warns about the missing wagon entity and does _not_
actually fix the issue.
How to test:
* In a world with both advtrains_train_subway and advtrains_train_japan
enabled, place a subway wagon, a Japanese engine, and a regular
Japanese wagon.
* Add the test mod to the world; do NOT remove advtrains_train_japan.
* Restart the world. Notice that the Japanese wagons still appear as
Japanese wagons despite being aliased to subway wagons.
* Restart the world without the advtrains_train_japan mod. Notice that
the engine appears as the subway wagon while the regular Japanese
wagon appears as the wagon placeholder. [v2]: Also note that the
warning message about the missing wagon prototype still mentions the
regular Japanese wagon.
* Restart the world again with the advtrains_train_japan mod. Notice
that both type of Japanese wagons reappear as Japanese wagons.
* Observe that unittests work.
Test mod:
advtrains.register_wagon_alias("advtrains:engine_japan", "advtrains:subway_wagon")
advtrains.register_wagon_alias("advtrains:wagon_japan", "advtrains:wagon_japan")
-rw-r--r-- | advtrains/spec/wagons_spec.lua | 40 | ||||
-rw-r--r-- | advtrains/trainlogic.lua | 13 | ||||
-rw-r--r-- | advtrains/wagons.lua | 35 |
3 files changed, 74 insertions, 14 deletions
diff --git a/advtrains/spec/wagons_spec.lua b/advtrains/spec/wagons_spec.lua new file mode 100644 index 0000000..df0687b --- /dev/null +++ b/advtrains/spec/wagons_spec.lua @@ -0,0 +1,40 @@ +require "mineunit" +mineunit "core" + +_G.advtrains = { + wagon_load_range = 32 +} +sourcefile "wagons" + +local myproto = {_test = true} +advtrains.register_wagon(":mywagon", myproto, "My wagon", "", false) +advtrains.register_wagon_alias(":myalias", ":mywagon") +advtrains.register_wagon_alias(":myotheralias", ":myalias") + +local myotherproto = {_other = true} +advtrains.register_wagon(":noalias", myotherproto, "Not aliased wagon", "", false) +advtrains.register_wagon_alias(":noalias", ":mywagon") + +advtrains.register_wagon_alias(":nilalias", ":nil") + +advtrains.register_wagon_alias(":R1", ":R2") +advtrains.register_wagon_alias(":R2", ":R3") +advtrains.register_wagon_alias(":R3", ":R1") + +describe("wagon alias system", function() + it("should work", function() + assert.same({":mywagon", myproto}, {advtrains.resolve_wagon_alias(":myalias")}) + assert.equal(myproto, advtrains.wagon_prototypes[":myalias"]) + assert.same({":mywagon", myproto}, {advtrains.resolve_wagon_alias(":myotheralias")}) + end) + it("should respect wagon registration", function() + assert.same({":noalias", myotherproto}, {advtrains.resolve_wagon_alias(":noalias")}) + end) + it("should handle recursive loops", function() + assert.same({}, {advtrains.resolve_wagon_alias(":R1")}) + end) + it("should return nil for missing entries", function() + assert.same({}, {advtrains.resolve_wagon_alias(":what")}) + assert.same({}, {advtrains.resolve_wagon_alias(":nilalias")}) + end) +end) diff --git a/advtrains/trainlogic.lua b/advtrains/trainlogic.lua index 796aae9..3b006d2 100644 --- a/advtrains/trainlogic.lua +++ b/advtrains/trainlogic.lua @@ -1113,8 +1113,17 @@ function advtrains.spawn_wagons(train_id) if advtrains.position_in_range(pos, ablkrng) then --atdebug("wagon",w_id,"spawning") local wt = advtrains.get_wagon_prototype(data) - local wagon = minetest.add_entity(pos, wt):get_luaentity() - wagon:set_id(w_id) + local wobj = minetest.add_entity(pos, wt) + if not wobj then + atwarn("Failed to spawn wagon", w_id, "of type", wt) + else + local wagon = wobj:get_luaentity() + if not wagon then + atwarn("Wagon", w_id, "of type", wt, "spawned with nil luaentity") + else + wagon:set_id(w_id) + end + end end end else diff --git a/advtrains/wagons.lua b/advtrains/wagons.lua index 50ce4ab..8bb12ee 100644 --- a/advtrains/wagons.lua +++ b/advtrains/wagons.lua @@ -16,15 +16,8 @@ advtrains.wagons = {} advtrains.wagon_alias = {} advtrains.wagon_prototypes = setmetatable({}, { __index = function(t, k) - local rtn_val = rawget(t, k) - if rtn_val ~= nil then - return rtn_val - end - local alias = advtrains.wagon_alias[k] - if alias then - return rawget(t, alias) - end - return nil + local _, proto = advtrains.resolve_wagon_alias(k) + return proto end }) advtrains.wagon_objects = {} @@ -1338,17 +1331,35 @@ function advtrains.get_wagon_prototype(data) data.type = data.entity_name data.entity_name = nil end - if not wt or not advtrains.wagon_prototypes[wt] then + local rt, proto = advtrains.resolve_wagon_alias(wt) + if not rt then atwarn("Unable to load wagon type",wt,", using placeholder") - wt="advtrains:wagon_placeholder" + rt = "advtrains:wagon_placeholder" + proto = advtrains.wagon_prototypes[rt] end - return wt, advtrains.wagon_prototypes[wt] + return rt, proto end function advtrains.register_wagon_alias(src, dst) advtrains.wagon_alias[src] = dst end +local function recursive_resolve_alias(name, seen) + local prototype = rawget(advtrains.wagon_prototypes, name) + if prototype then + return name, prototype + end + local resolved = advtrains.wagon_alias[name] + if resolved and not seen[resolved] then + seen[name] = true + return recursive_resolve_alias(resolved, seen) + end +end + +function advtrains.resolve_wagon_alias(name) + return recursive_resolve_alias(name, {}) +end + function advtrains.standard_inventory_formspec(self, pname, invname) --[[minetest.chat_send_player(pname, string.format("self=%s, pname=%s, invname=%s", self, pname, invname)) for k,v in pairs(self) do |