Add proper UI aspect selection for static dialog

5 files changed, 111 insertions, 61 deletions

diff --git a/advtrains/nodedb.lua b/advtrains/nodedb.lua
index ff07df4..62405ce 100644
--- a/advtrains/nodedb.lua
+++ b/advtrains/nodedb.lua
@@ -212,6 +212,10 @@ function ndb.get_node(pos)
 	end
 	return n
 end
+function ndb.get_ndef(pos)
+	local n=ndb.get_node_or_nil(pos)
+	return n and minetest.registered_nodes[n.name]
+end
 function ndb.get_node_raw(pos)
 	local cid=ndbget(pos.x, pos.y, pos.z)
 	if cid then
diff --git a/advtrains_interlocking/routesetting.lua b/advtrains_interlocking/routesetting.lua
index a576139..51709dc 100644
--- a/advtrains_interlocking/routesetting.lua
+++ b/advtrains_interlocking/routesetting.lua
@@ -159,15 +159,8 @@ function ilrs.set_route(signal, route, try)
 		end
 	end
 	for i = #signals, 1, -1 do
-		if lastsig then
-			local tcbs = signals[i]
-			local pos = tcbs.signal
-			local _, assigned_by = advtrains.distant.get_main(pos)
-			if (not nodst) and (not assigned_by or assigned_by == "routesetting") then
-				advtrains.distant.assign(lastsig, pos, "routesetting", true)
-			end
-			advtrains.interlocking.signal.update_route_aspect(tcbs, i ~= 1)
-		end
+		-- TODO add logic for distant signal assign
+		advtrains.interlocking.signal.update_route_aspect(signals[i], i ~= 1)
 	end
 	
 	return true
diff --git a/advtrains_interlocking/signal_api.lua b/advtrains_interlocking/signal_api.lua
index 5216594..7826d30 100644
--- a/advtrains_interlocking/signal_api.lua
+++ b/advtrains_interlocking/signal_api.lua
@@ -5,13 +5,14 @@ local F = advtrains.formspec
 local signal = {}
 
 signal.MASP_HALT = {
-	name = nil,
-	speed = nil,
+	name = "_halt",
+	speed = 0,
+	halt = true,
 	remote = nil,
 }
 
 signal.MASP_FREE = {
-	name = "_free",
+	name = "_default",
 	speed = -1,
 	remote = nil,
 }
@@ -44,9 +45,9 @@ b) the distant signal's aspect group name & aspect table
 One concrete combination of lights/shapes that a signal signal shows. Handling these is at the discretion of
 the signal mod defining the signal, and they are typically combinations of main aspect and distant aspect
 Example:
-- A Ks signal has the aspect_group="proceed_12" set for a route
-- The signal at the end of the route shows aspect_group="proceed_8", advtrains also passes on that this means {main=8, shunt=false}
-- The ndef.advtrains.apply_aspect(pos, asp_group, dst_aspgrp, dst_aspinfo) determines that the signal should now show
+- A Ks signal has the main_aspect="proceed_12" set for a route
+- The signal at the end of the route shows main_aspect="proceed_8", advtrains also passes on that this means {main=8, shunt=false}
+- The ndef.afunction(pos, node, main_aspect, rem_aspect, rem_aspinfo) determines that the signal should now show
 		blinking green with main indicator 12 and dst indicator 8, and sets the nodes accordingly.
 		This function can now return the Aspect Info table, which will be cached by advtrains until the aspect changes again
 		and will be used when a train approaches the signal. If nil is returned, then the aspect will be queried next time
@@ -56,10 +57,14 @@ Note that once apply_aspect returns, there is no need for advtrains anymore to q
 When the signal, for any reason, wants to change its aspect by itself *without* going through the signal API then
 it should update the aspect info cache by calling advtrains.interlocking.signal.update_aspect_info(pos)
 
-Apply_aspect may also receive nil as the main aspect. It usually means that the signal is not assigned to anything particular,
+Apply_aspect may also receive the special main aspect { name = "_halt", halt = true }. It usually means that the signal is not assigned to anything particular,
 and it should cause the signal to show its most restrictive aspect. Typically it is a halt aspect, but e.g. for distant-only
 signals this would be "expect stop".
 
+A special case occurs for pure distant signals: Such signals must set apply_aspect, but must not set main_aspects. Behavior is as follows:
+- Signal is uninitialized, distant signal is not assigned to a main signal, or no route is set: main_aspect == { name = "_halt", halt = true } and rem_aspect == nil
+- A remote main signal is assigned (either by user or by route): main_aspect is always { name = "_default" } and rem_aspect / rem_aspinfo give the correct information
+
 Main aspect names starting with underscore (e.g. "_default") are reserved and must not be used!
 
 == Aspect Info ==
@@ -90,6 +95,7 @@ ndef.advtrains = {
 	apply_aspect = function(pos, node, main_aspect, rem_aspect, rem_aspinfo)
 		-- set the node to show the desired aspect
 		-- called by advtrains when this signal's aspect group or the remote signal's aspect changes
+		-- main_aspect is never nil, but can be one of the special aspects { name = "_halt", halt = true } or { name = "_default" }
 		-- MAY return the aspect_info. If it returns nil then get_aspect_info will be queried at a later point.
 	get_aspect_info(pos, main_aspect)
 		-- Returns the aspect info table (main, shunt, dst etc.)
@@ -268,36 +274,41 @@ end
 function signal.get_aspect_internal(pos, aspt)
 	if not aspt then
 		-- oh, no main aspect, nevermind
-		return nil, nil, nil
+		return signal.MASP_HALT, nil, nil
 	end
 	atdebug("get_aspect_internal",pos,aspt)
 	-- look aspect in nodedef
 	local node = advtrains.ndb.get_node_or_nil(pos)
 	local ndef = node and minetest.registered_nodes[node.name]
 	local ndefat = ndef and ndef.advtrains
-	-- only if signal defines main aspect and its set in aspt
-	if ndefat and ndefat.main_aspects and aspt.name then
-		if not ndefat.main_aspects_lookup then
-			cache_mainaspects(ndefat)
-		end
-		local masp = ndefat.main_aspects_lookup[aspt.name]
-		-- special handling for the default free aspect ("_free")
-		if aspt.name == "_free" then
-			masp = ndefat.main_aspects[1]
-		end
-		if not masp then
-			atwarn(pos,"invalid main aspect",aspt.name,"valid are",ndefat.main_aspects_lookup)
-			return nil, aspt.remote, node, ndef
-		end
-		-- if speed, then apply speed
-		if masp.speed and aspt.speed then
-			masp = table.copy(masp)
-			masp.speed = aspt.speed
+	if ndefat and ndefat.apply_aspect then
+		-- only if signal defines main aspect and its set in aspt
+		if ndefat.main_aspects and aspt.name then
+			if not ndefat.main_aspects_lookup then
+				cache_mainaspects(ndefat)
+			end
+			local masp = ndefat.main_aspects_lookup[aspt.name]
+			-- special handling for the default free aspect ("_default")
+			if aspt.name == "_default" then
+				masp = ndefat.main_aspects[1]
+			end
+			if not masp then
+				atwarn(pos,"invalid main aspect",aspt.name,"valid are",ndefat.main_aspects_lookup)
+				return signal.MASP_HALT, aspt.remote, node, ndef
+			end
+			-- if speed, then apply speed
+			if masp.speed and aspt.speed then
+				masp = table.copy(masp)
+				masp.speed = aspt.speed
+			end
+			return masp, aspt.remote, node, ndef
+		elseif aspt.name then
+			-- Distant-only signal, still supports kind of default aspect
+			return { name = aspt.name, speed = aspt.speed }, aspt.remote, node, ndef
 		end
-		return masp, aspt.remote, node, ndef
 	end
-	-- invalid node or no main aspect, return nil for masp
-	return nil, aspt.remote, node, ndef
+	-- invalid node or no main aspect, return default halt aspect for masp
+	return signal.MASP_HALT, aspt.remote, node, ndef
 end
 
 -- For the signal at pos, get the "aspect info" table. This contains the speed signalling information at this location
@@ -327,7 +338,6 @@ function signal.reapply_aspect(pts)
 	end
 	-- resolve mainaspect table by name
 	local pos = advtrains.decode_pos(pts)
-	-- note: masp may be nil, when aspt.name was nil. Valid case for distant-only signals
 	local masp, remote, node, ndef = signal.get_aspect_internal(pos, aspt)
 	-- if we have remote, resolve remote
 	local rem_masp, rem_aspi
diff --git a/advtrains_interlocking/signal_aspect_ui.lua b/advtrains_interlocking/signal_aspect_ui.lua
index e5d2003..785e6d4 100644
--- a/advtrains_interlocking/signal_aspect_ui.lua
+++ b/advtrains_interlocking/signal_aspect_ui.lua
@@ -75,14 +75,34 @@ function advtrains.interlocking.show_ip_sa_form(pos, pname)
 	end
 	local ipform = advtrains.interlocking.make_ip_formspec_component(pos, 0.5, 0.5, 7)
 	local ma, rpos = advtrains.interlocking.signal.get_aspect(pos)
-	local saform = F.S_button_exit(0, 2, 4, "sa_dst_assign", rpos and minetest.pos_to_string(rpos) or "<distant signal>")
-			.. F.S_button_exit(0, 3, 2, "sa_tmp_mainfree", "Main to free") .. F.S_button_exit(2, 3, 2, "sa_tmp_mainhalt", "Main to halt")
 	local form = {
 		"formspec_version[4]",
-		"size[8,4]",
+		"size[8,4.5]",
 		ipform,
-		saform,
 	}
+	-- Create Signal aspect formspec elements
+	local ndef = advtrains.ndb.get_ndef(pos)
+	if ndef and ndef.advtrains then
+		-- main aspect list
+		if ndef.advtrains.main_aspects then
+			local entries = { "<none>" }
+			local sel = 1
+			for i, mae in ipairs(ndef.advtrains.main_aspects) do
+				entries[i+1] = mae.description
+				if ma and ma.name == mae.name then
+					sel = i+1
+				end
+			end
+			form[#form+1] = F.dropdown(0.5, 2.5, 4, "sa_mainaspect", entries, sel, true)
+		end
+		-- distant signal assign (is shown either when main_aspect is not none, or when pure distant signal)
+		if rpos then
+			form[#form+1] = F.button_exit(0.5, 3.5, 4, "sa_undistant", "Dst: " .. minetest.pos_to_string(rpos))
+		elseif (ma and not ma.halt) or not ndef.advtrains.main_aspects then
+			form[#form+1] = F.button_exit(0.5, 3.5, 4, "sa_distant", "<assign distant>")
+		end
+	end
+	
 	minetest.show_formspec(pname, "at_il_ipsaform_"..minetest.pos_to_string(pos), table.concat(form))
 end
 
@@ -90,18 +110,42 @@ function advtrains.interlocking.handle_ip_sa_formspec_fields(pname, pos, fields)
 	if not (pos and minetest.check_player_privs(pname, {train_operator=true, interlocking=true})) then
 		return
 	end
+	local ma, rpos = advtrains.interlocking.signal.get_aspect(pos)
+	-- mainaspect dropdown
+	if fields.sa_mainaspect then
+		local idx = tonumber(fields.sa_mainaspect)
+		local new_ma = nil
+		if idx > 1 then
+			local ndef = advtrains.ndb.get_ndef(pos)
+			if ndef and ndef.advtrains and ndef.advtrains.main_aspects then
+				new_ma = ndef.advtrains.main_aspects[idx - 1]
+			end
+		end
+		if new_ma and (new_ma.name ~= ma.name or new_ma.speed ~= ma.speed) then
+			advtrains.interlocking.signal.set_aspect(pos, new_ma.name, new_ma.speed, rpos)
+		elseif not new_ma then
+			-- reset everything
+			advtrains.interlocking.signal.set_aspect(pos, nil, nil, nil)
+		end
+		
+	end
+	-- buttons
 	if fields.ip_set then
 		advtrains.interlocking.init_ip_assign(pos, pname)
+		return
 	elseif fields.ip_clear then
 		advtrains.interlocking.db.clear_ip_by_signalpos(pos)
-	elseif fields.sa_dst_assign then
+		return
+	elseif fields.sa_distant then
 		advtrains.interlocking.init_distant_assign(pos, pname)
-	elseif fields.sa_tmp_mainfree then
-		local ma, rpos = advtrains.interlocking.signal.get_aspect(pos)
-		advtrains.interlocking.signal.set_aspect(pos, "_free", -1, rpos)
-	elseif fields.sa_tmp_mainhalt then
-		local ma, rpos = advtrains.interlocking.signal.get_aspect(pos)
-		advtrains.interlocking.signal.set_aspect(pos, nil, nil, rpos)
+		return
+	elseif fields.sa_undistant then
+		advtrains.interlocking.signal.set_aspect(pos, ma.name, ma.speed, nil)
+		return
+	end
+	-- show the form again unless one of the buttons was clicked
+	if not fields.quit then
+		advtrains.interlocking.show_ip_sa_form(pos, pname)
 	end
 end
 
@@ -180,8 +224,13 @@ minetest.register_on_punchnode(function(pos, node, player, pointed_thing)
 		local ma, rpos = advtrains.interlocking.signal.get_aspect(signalpos)
 		-- if punched pos is valid signal then set it as the new remote, otherwise nil
 		local nrpos
-		if advtrains.interlocking.signal.get_signal_cap_level(pos) > 1 then nrpos = pos end
-		advtrains.interlocking.signal.set_aspect(signalpos, ma.name, ma.speed, nrpos)
+		if advtrains.interlocking.signal.get_signal_cap_level(pos) > 1 then
+			nrpos = pos
+			if not ma then -- make sure that dst is never set without a main aspect (esp. for pure distant signal case)
+				ma = { name = "_default" }
+			end
+			advtrains.interlocking.signal.set_aspect(signalpos, ma.name, ma.speed, nrpos)
+		end
 		players_assign_distant[pname] = nil
 	end
 end)
diff --git a/advtrains_signals_ks/init.lua b/advtrains_signals_ks/init.lua
index abfb194..9cb0262 100755
--- a/advtrains_signals_ks/init.lua
+++ b/advtrains_signals_ks/init.lua
@@ -50,7 +50,7 @@ end
 
 local applyaspectf_main = function(rot)
  return function(pos, node, main_aspect, dst_aspect, dst_aspect_info)
-	if not main_aspect then
+	if main_aspect.halt then
 		-- halt aspect, set red and don't do anything further
 		advtrains.ndb.swap_node(pos, {name="advtrains_signals_ks:hs_danger_"..rot, param2 = node.param2})
 		setzs3v(pos, nil, rot)
@@ -119,20 +119,14 @@ local mainaspects_main = {
 		description = "Proceed (speed 4)",
 		zs3 = "4",
 	},
-	{
-		name = "halt",
-		description = "Halt",
-		zs3 = "off",
-		halt = true,
-	},
 }
 
 --Rangiersignal
 local applyaspectf_ra = function(rot)
  -- we get here the full main_aspect table
  return function(pos, node, main_aspect, dst_aspect, dst_aspect_info)
-	if main_aspect then
-		-- any main aspect is fine, there's only one anyway
+	if not main_aspect.halt then
+		-- any non-halt main aspect is fine, there's only one anyway
 		advtrains.ndb.swap_node(pos, {name="advtrains_signals_ks:ra_shuntd_"..rot, param2 = node.param2})
 	else
 		advtrains.ndb.swap_node(pos, {name="advtrains_signals_ks:ra_danger_"..rot, param2 = node.param2})