~gpcf/advtrains-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch

[PATCH v2] Address wagon aliasing issues

Details
Message ID
<20240919183733.396832-2-y5nw@protonmail.com>
DKIM signature
pass
Download raw message
Patch: +74 -14
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")

---
 advtrains/spec/wagons_spec.lua | 40 ++++++++++++++++++++++++++++++++++
 advtrains/trainlogic.lua       | 13 +++++++++--
 advtrains/wagons.lua           | 35 +++++++++++++++++++----------
 3 files changed, 74 insertions(+), 14 deletions(-)
 create mode 100644 advtrains/spec/wagons_spec.lua

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 f136577..4db349a 100644
--- a/advtrains/trainlogic.lua
+++ b/advtrains/trainlogic.lua
@@ -1117,8 +1117,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 536c8d4..144294e 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 = {}
@@ -1332,17 +1325,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
-- 
2.46.0
Reply to thread Export thread (mbox)