Distant signaling: avoid signal signs

5 files changed, 54 insertions, 6 deletions

diff --git a/advtrains_interlocking/distant.lua b/advtrains_interlocking/distant.lua
index 726c2b3..4175875 100644
--- a/advtrains_interlocking/distant.lua
+++ b/advtrains_interlocking/distant.lua
@@ -85,6 +85,23 @@ local function unassign_all(pos, force)
 	unassign_dst(pos, force)
 end
 
+--- Check whether a signal is "appropriate" for the distant signal system.
+-- Currently, a signal is considered appropriate if its signal aspect can be set.
+-- @function appropriate_signal
+-- @param pos The position of the signal
+local function appropriate_signal(pos)
+	local node = advtrains.ndb.get_node(pos)
+	local ndef = minetest.registered_nodes[node.name] or {}
+	if not ndef then
+		return false
+	end
+	local atdef = ndef.advtrains
+	if not atdef then
+		return false
+	end
+	return atdef.supported_aspects and atdef.set_aspect and true
+end
+
 --- Assign a distant signal to a main signal.
 -- @function assign
 -- @param main The position of the main signal.
@@ -92,6 +109,9 @@ end
 -- @param[opt="manual"] by The method of assignment.
 -- @param[opt=false] skip_update Whether to skip callbacks.
 local function assign(main, dst, by, skip_update)
+	if not (appropriate_signal(main) and appropriate_signal(dst)) then
+		return
+	end
 	local pts_main = pts(main)
 	local pts_dst = pts(dst)
 	local t = db_distant[pts_main]
@@ -177,4 +197,5 @@ advtrains.distant = {
 	update_main = update_main,
 	update_dst = update_dst,
 	update_signal = update_signal,
+	appropriate_signal = appropriate_signal,
 }
diff --git a/advtrains_interlocking/distant_ui.lua b/advtrains_interlocking/distant_ui.lua
index 0907684..bb66dc4 100644
--- a/advtrains_interlocking/distant_ui.lua
+++ b/advtrains_interlocking/distant_ui.lua
@@ -56,6 +56,10 @@ local function init_signal_assignment(pname, pos)
 		minetest.chat_send_player(pname, attrans("This operation is not allowed without the @1 privilege.", "interlocking"))
 		return
 	end
+	if not D.appropriate_signal(pos) then
+		minetest.chat_send_player(pname, attrans("Incompatible signal."))
+		return
+	end
 	signal_pos[pname] = pos
 	minetest.chat_send_player(pname, attrans("Please punch the signal to use as the main signal."))
 end
@@ -66,6 +70,10 @@ local function init_distant_assignment(pname, pos)
 		minetest.send_chat_player(pname, attrans("This operation is now allowed without the @1 privilege.", "interlocking"))
 		return
 	end
+	if not D.appropriate_signal(pos) then
+		minetest.chat_send_player(pname, attrans("Incompatible signal."))
+		return
+	end
 	distant_pos[pname] = pos
 	minetest.chat_send_player(pname, attrans("Please punch the signal to use as the distant signal."))
 end
@@ -87,7 +95,7 @@ minetest.register_on_punchnode(function(pos, node, player, pointed_thing)
 	signal_pos[pname] = nil
 	distant_pos[pname] = nil
 	local is_signal = minetest.get_item_group(node.name, "advtrains_signal") >= 2
-	if not is_signal then
+	if not (is_signal and D.appropriate_signal(pos)) then
 		minetest.chat_send_player(pname, attrans("Incompatible signal."))
 		return
 	end
diff --git a/advtrains_interlocking/signal_api.lua b/advtrains_interlocking/signal_api.lua
index cd408d7..1b4a21c 100644
--- a/advtrains_interlocking/signal_api.lua
+++ b/advtrains_interlocking/signal_api.lua
@@ -159,16 +159,20 @@ function advtrains.interlocking.show_ip_form(pos, pname, only_notset)
 		return
 	end
 	local ipform, pts, connid = advtrains.interlocking.make_ip_formspec_component(pos, 0.5, 0.5, 7)
-	local form = table.concat {
+	local form = {
 		"formspec_version[4]",
-		"size[8,6.75]",
+		"size[8,2.25]",
 		ipform,
-		advtrains.interlocking.make_dst_formspec_component(pos, 0.5, 2, 7, 4.25),
 	}
 	if pts then
 		local ipos = minetest.string_to_pos(pts)
 		ipmarker(ipos, connid)
 	end
+	if advtrains.distant.appropriate_signal(pos) then
+		form[#form+1] = advtrains.interlocking.make_dst_formspec_component(pos, 0.5, 2, 7, 4.25)
+		form[2] = "size[8,6.75]"
+	end
+	form = table.concat(form)
 	if not only_notset or not pts then
 		minetest.show_formspec(pname, "at_il_propassign_"..minetest.pos_to_string(pos), form)
 	end
diff --git a/advtrains_interlocking/signal_aspect_accessors.lua b/advtrains_interlocking/signal_aspect_accessors.lua
index e55814e..e419515 100644
--- a/advtrains_interlocking/signal_aspect_accessors.lua
+++ b/advtrains_interlocking/signal_aspect_accessors.lua
@@ -144,7 +144,7 @@ local function get_real_aspect(pos)
 	if ndef.advtrains and ndef.advtrains.get_aspect then
 		local asp = ndef.advtrains.get_aspect(pos, node) or I.DANGER
 		local suppasp = get_supported_aspects(pos)
-		if suppasp.type == 2 then
+		if suppasp and suppasp.type == 2 then
 			asp = A.type2_to_type1(suppasp, asp)
 		end
 		return adjust_aspect(pos, asp)
diff --git a/advtrains_interlocking/spec/basic_signalling_spec.lua b/advtrains_interlocking/spec/basic_signalling_spec.lua
index 720b274..cce0f15 100644
--- a/advtrains_interlocking/spec/basic_signalling_spec.lua
+++ b/advtrains_interlocking/spec/basic_signalling_spec.lua
@@ -23,6 +23,12 @@ sourcefile("signal_api")
 sourcefile("signal_aspect_accessors")
 fixture("../../demosignals")
 
+minetest.register_node("advtrains_interlocking:signal_sign", {
+	advtrains = {
+		get_aspcet = function() return {main = 19} end
+	}
+})
+
 local D = advtrains.distant
 local I = advtrains.interlocking
 
@@ -32,7 +38,7 @@ local stub_aspect_t1 = {
 	danger = {main = 0, shunt = false},
 }
 local stub_pos_t1 = {}
-for i = 1, 3 do
+for i = 1, 4 do
 	stub_pos_t1[i] = {x = 1, y = 0, z = i}
 end
 
@@ -40,6 +46,7 @@ world.layout {
 	{stub_pos_t1[1], "advtrains_interlocking:ds_danger"},
 	{stub_pos_t1[2], "advtrains_interlocking:ds_slow"},
 	{stub_pos_t1[3], "advtrains_interlocking:ds_free"},
+	{stub_pos_t1[4], "advtrains_interlocking:signal_sign"},
 }
 
 describe("API for supposed signal aspects", function()
@@ -84,4 +91,12 @@ describe("Distant signaling", function()
 		assert.same({}, {D.get_main(stub_pos_t1[3])})
 		assert.same(stub_aspect_t1.free, I.signal_get_aspect(stub_pos_t1[3]))
 	end)
+	it("should reject signal signs", function()
+		D.assign(stub_pos_t1[1], stub_pos_t1[4])
+		assert.same({}, D.get_dst(stub_pos_t1[1]))
+		assert.same({}, {D.get_main(stub_pos_t1[4])})
+		D.assign(stub_pos_t1[4], stub_pos_t1[1])
+		assert.same({}, D.get_dst(stub_pos_t1[4]))
+		assert.same({}, {D.get_main(stub_pos_t1[1])})
+	end)
 end)