Miscellaneous routesetting fixes

- Move handling of "route_committed" to the routesetting function
- Put aspect in every TCBS on the way
- Add "route_origin" to TCBS fields to prevent subroute cancelling
- Cancel entire route when another train enters from the wrong TCB

5 files changed, 115 insertions, 57 deletions

diff --git a/advtrains_interlocking/database.lua b/advtrains_interlocking/database.lua
index a2df111..299f6cc 100644
--- a/advtrains_interlocking/database.lua
+++ b/advtrains_interlocking/database.lua
@@ -248,9 +248,7 @@ end
 
 
 -- various helper functions handling sigd's
-local function sigd_equal(sigd, cmp)
-	return vector.equals(sigd.p, cmp.p) and sigd.s==cmp.s
-end
+local sigd_equal = advtrains.interlocking.sigd_equal
 local function insert_sigd_nodouble(list, sigd)
 	for idx, cmp in pairs(list) do
 		if sigd_equal(sigd, cmp) then
diff --git a/advtrains_interlocking/init.lua b/advtrains_interlocking/init.lua
index 1a0929d..7239674 100644
--- a/advtrains_interlocking/init.lua
+++ b/advtrains_interlocking/init.lua
@@ -3,6 +3,11 @@
 
 advtrains.interlocking = {}
 
+function advtrains.interlocking.sigd_equal(sigd, cmp)
+	return vector.equals(sigd.p, cmp.p) and sigd.s==cmp.s
+end
+
+
 local modpath = minetest.get_modpath(minetest.get_current_modname()) .. DIR_DELIM
 
 dofile(modpath.."database.lua")
diff --git a/advtrains_interlocking/routesetting.lua b/advtrains_interlocking/routesetting.lua
index 7dc6672..c6706f7 100644
--- a/advtrains_interlocking/routesetting.lua
+++ b/advtrains_interlocking/routesetting.lua
@@ -6,9 +6,26 @@ local function sigd_to_string(sigd)
 	return minetest.pos_to_string(sigd.p).." / "..lntrans[sigd.s]
 end
 
+local asp_generic_free = {
+	main = {
+		free = true,
+		speed = -1,
+	},
+	shunt = {
+		free = false,
+	},
+	dst = {
+		free = true,
+		speed = -1,
+	},
+	info = {}
+}
+
 local ildb = advtrains.interlocking.db
 local ilrs = {}
 
+local sigd_equal = advtrains.interlocking.sigd_equal
+
 -- table containing locked points
 -- also manual locks (maintenance a.s.o.) are recorded here
 -- [pts] = { 
@@ -88,6 +105,11 @@ function ilrs.set_route(signal, route, try)
 		end
 		-- reserve ts and write locks
 		if not try then
+			local nvar = c_rseg.next
+			if not route[i+1] then
+				-- We shouldn't use the "next" value of the final route segment, because this can lead to accidental route-cancelling of already set routes from another signal.
+				nvar = nil
+			end
 			c_ts.route = {
 				origin = signal,
 				entry = c_sigd,
@@ -96,8 +118,14 @@ function ilrs.set_route(signal, route, try)
 			}
 			c_ts.route_post = {
 				locks = c_lckp,
-				next = c_rseg.next,
+				next = nvar,
 			}
+			if c_tcbs.signal then
+				c_tcbs.route_committed = true
+				c_tcbs.aspect = asp_generic_free
+				c_tcbs.route_origin = signal
+				advtrains.interlocking.update_signal_aspect(c_tcbs)
+			end
 		end
 		-- advance
 		first = nil
@@ -185,9 +213,6 @@ function ilrs.remove_route_locks(pts, nocallbacks)
 	end
 end
 
-local function sigd_equal(sigd, cmp)
-	return vector.equals(sigd.p, cmp.p) and sigd.s==cmp.s
-end
 
 -- starting from the designated sigd, clears all subsequent route and route_post
 -- information from the track sections.
@@ -198,6 +223,7 @@ function ilrs.cancel_route_from(sigd)
 	local c_sigd = sigd
 	local c_tcbs, c_ts_id, c_ts, c_rseg, c_lckp
 	while c_sigd do
+		--atdebug("cancel_route_from: at sigd",c_sigd)
 		c_tcbs = ildb.get_tcbs(c_sigd)
 		c_ts_id = c_tcbs.ts_id
 		c_ts = ildb.get_ts(c_ts_id)
@@ -205,9 +231,20 @@ function ilrs.cancel_route_from(sigd)
 		if not c_ts
 			or not c_ts.route
 			or not sigd_equal(c_ts.route.entry, c_sigd) then
+			--atdebug("cancel_route_from: abort (eoi/no route):")
 			return
 		end
 		
+		--atdebug("cancelling",c_ts.route.rsn)
+		-- clear signal aspect and routesetting state
+		c_tcbs.route_committed = nil
+		c_tcbs.aspect = nil
+		c_tcbs.routeset = nil
+		c_tcbs.route_auto = nil
+		c_tcbs.route_origin = nil
+		
+		advtrains.interlocking.update_signal_aspect(c_tcbs)
+		
 		c_ts.route = nil
 		
 		if c_ts.route_post then
@@ -219,37 +256,30 @@ function ilrs.cancel_route_from(sigd)
 		c_ts.route_post = nil
 		minetest.after(0, advtrains.interlocking.route.update_waiting, "ts", c_ts_id)
 	end
+	--atdebug("cancel_route_from: done (no final sigd)")
 end
 
-local asp_generic_free = {
-	main = {
-		free = true,
-		speed = -1,
-	},
-	shunt = {
-		free = false,
-	},
-	dst = {
-		free = true,
-		speed = -1,
-	},
-	info = {}
-}
-
 -- TCBS Routesetting helper: generic update function for
 -- route setting
 
 function ilrs.update_route(sigd, tcbs, newrte, cancel)
 	-- in general, always show danger signal
 	tcbs.aspect = nil
+	--atdebug("Update_Route for",sigd,tcbs.signal_name)
+	if tcbs.route_origin and not sigd_equal(tcbs.route_origin, sigd) then
+		--atdebug("Signal not in control, held by",tcbs.signal_name)
+		return
+	end
 	if (newrte and tcbs.routeset and tcbs.routeset ~= newrte) or cancel then
 		if tcbs.route_committed then
 			--atdebug("Cancelling:",tcbs.routeset)
 			advtrains.interlocking.route.cancel_route_from(sigd)
 		end
 		tcbs.route_committed = nil
-		tcbs.routeset = newrte
+		tcbs.aspect = nil
+		tcbs.routeset = nil
 		tcbs.route_auto = nil
+		tcbs.route_rsn = nil
 	end
 	if newrte or tcbs.routeset then
 		if newrte then tcbs.routeset = newrte end
@@ -271,9 +301,6 @@ function ilrs.update_route(sigd, tcbs, newrte, cancel)
 			end
 		else
 			--atdebug("Committed Route:",tcbs.routeset)
-			tcbs.route_committed = true
-			tcbs.route_rsn = false
-			tcbs.aspect = asp_generic_free
 		end
 	end
 	advtrains.interlocking.update_signal_aspect(tcbs)
diff --git a/advtrains_interlocking/tcb_ts_ui.lua b/advtrains_interlocking/tcb_ts_ui.lua
index 450c756..6e538c5 100644
--- a/advtrains_interlocking/tcb_ts_ui.lua
+++ b/advtrains_interlocking/tcb_ts_ui.lua
@@ -7,6 +7,8 @@ local players_link_ts = {}
 local ildb = advtrains.interlocking.db
 local ilrs = advtrains.interlocking.route
 
+local sigd_equal = advtrains.interlocking.sigd_equal
+
 local lntrans = { "A", "B" }
 
 local function sigd_to_string(sigd)
@@ -337,7 +339,7 @@ function advtrains.interlocking.show_ts_form(ts_id, pname, sel_tcb)
 	if ts.route then
 		form = form.."label[0.5,6.1;Route is set: "..ts.route.rsn.."]"
 	elseif ts.route_post then
-		form = form.."label[0.5,6.1;Section holds "..#ts.route_post.lcks.." route locks.]"
+		form = form.."label[0.5,6.1;Section holds "..#(ts.route_post.lcks or {}).." route locks.]"
 	end
 	-- occupying trains
 	if ts.trains and #ts.trains>0 then
@@ -558,28 +560,41 @@ function advtrains.interlocking.show_signalling_form(sigd, pname, sel_rte)
 		
 		form = form.."button[0.5,6;  5,1;cancelroute;Cancel Route]"
 	else
-		local strtab = {}
-		for idx, route in ipairs(tcbs.routes) do
-			strtab[#strtab+1] = minetest.formspec_escape(route.name)
-		end
-		form = form.."label[0.5,2.5;Routes:]"
-		form = form.."textlist[0.5,3;5,3;rtelist;"..table.concat(strtab, ",").."]"
-		if sel_rte then
-			form = form.."button[0.5,6;  5,1;setroute;Set Route]"
-			form = form.."button[0.5,7;2,1;dsproute;Show]"
+		if not tcbs.route_origin then
+			local strtab = {}
+			for idx, route in ipairs(tcbs.routes) do
+				strtab[#strtab+1] = minetest.formspec_escape(route.name)
+			end
+			form = form.."label[0.5,2.5;Routes:]"
+			form = form.."textlist[0.5,3;5,3;rtelist;"..table.concat(strtab, ",").."]"
+			if sel_rte then
+				form = form.."button[0.5,6;  5,1;setroute;Set Route]"
+				form = form.."button[0.5,7;2,1;dsproute;Show]"
+				if hasprivs then
+					form = form.."button[2.5,7;1,1;delroute;Delete]"
+					form = form.."button[3.5,7;2,1;editroute;Edit]"
+				end
+			end
 			if hasprivs then
-				form = form.."button[2.5,7;1,1;delroute;Delete]"
-				form = form.."button[3.5,7;2,1;editroute;Edit]"
+				form = form.."button[0.5,8;2.5,1;newroute;New Route]"
+				form = form.."button[  3,8;2.5,1;unassign;Unassign Signal]"
+				form = form.."button[  3,9;2.5,1;influp;Influence Point]"
 			end
-		end
-		if hasprivs then
-			form = form.."button[0.5,8;2.5,1;newroute;New Route]"
-			form = form.."button[  3,8;2.5,1;unassign;Unassign Signal]"
-			form = form.."button[  3,9;2.5,1;influp;Influence Point]"
+		elseif sigd_equal(tcbs.route_origin, sigd) then
+			-- something has gone wrong: tcbs.routeset should have been set...
+			atwarn("Signal",tcbs.signal_name,"- Unknown route set. Route is being cancelled.")
+			ilrs.cancel_route_from(sigd)
+			return
+		else
+			form = form.."label[0.5,2.5;Route is set over this signal by:\n"..sigd_to_string(tcbs.route_origin).."]"
+			form = form.."label[0.5,4;Wait for this route to be cancelled in order to do anything here.]"
 		end
 	end	
 	sig_pselidx[pname] = sel_rte
 	minetest.show_formspec(pname, "at_il_signalling_"..minetest.pos_to_string(sigd.p).."_"..sigd.s, form)
+	
+	-- always a good idea to update the signal aspect
+	advtrains.interlocking.update_signal_aspect(tcbs)
 end
 
 
diff --git a/advtrains_interlocking/train_sections.lua b/advtrains_interlocking/train_sections.lua
index 8bc9716..c0a911e 100644
--- a/advtrains_interlocking/train_sections.lua
+++ b/advtrains_interlocking/train_sections.lua
@@ -21,6 +21,7 @@ It will be possible to indicate a section "free" via the GUI.
 
 local ildb = advtrains.interlocking.db
 
+local sigd_equal = advtrains.interlocking.sigd_equal
 
 local function itexist(tbl, com)
 	for _,item in ipairs(tbl) do
@@ -60,7 +61,7 @@ local function itkremove(tbl, ikey, com)
 	end
 end
 
-local function setsection(tid, train, ts_id, ts, origin)
+local function setsection(tid, train, ts_id, ts, sigd)
 	-- train
 	if not train.il_sections then train.il_sections = {} end
 	if not itkexist(train.il_sections, "ts_id", ts_id) then
@@ -73,25 +74,37 @@ local function setsection(tid, train, ts_id, ts, origin)
 		table.insert(ts.trains, tid)
 	end
 	
+	-- routes
+	local tcbs = advtrains.interlocking.db.get_tcbs(sigd)
+	
 	-- route setting - clear route state
 	if ts.route then
-		if ts.route.first then
-			-- this is the first route section. clear route status from origin sigd
-			local tcbs = advtrains.interlocking.db.get_tcbs(ts.route.origin)
-			if tcbs then
-				tcbs.route_committed = nil
-				tcbs.aspect = nil
-				advtrains.interlocking.update_signal_aspect(tcbs)
-				if tcbs.route_auto then
-					advtrains.interlocking.route.update_route(ts.route.origin, tcbs)
-				else
-					tcbs.routeset = nil
-				end
+		--atdebug(tid,"enters",ts_id,"examining Routestate",ts.route)
+		if not sigd_equal(ts.route.entry, sigd) then
+			-- Train entered not from the route. Locate origin and cancel route!
+			atwarn("Train",tid,"hit route",ts.route.rsn,"!")
+			advtrains.interlocking.route.cancel_route_from(ts.route.origin)
+			atwarn("Route was cancelled.")
+		end
+		-- train entered route regularily. Reset route and signal
+		tcbs.route_committed = nil
+		tcbs.route_comitted = nil -- TODO compatibility cleanup
+		tcbs.aspect = nil
+		tcbs.route_origin = nil
+		advtrains.interlocking.update_signal_aspect(tcbs)
+		if tcbs.signal and sigd_equal(ts.route.entry, ts.route.origin) then
+			if tcbs.route_auto and tcbs.routeset then
+				--atdebug("Resetting route (",ts.route.origin,")")
+				advtrains.interlocking.route.update_route(ts.route.origin, tcbs)
+			else
+				tcbs.routeset = nil
 			end
 		end
 		ts.route = nil
 	end
-	
+	if tcbs.signal then
+		advtrains.interlocking.route.update_route(sigd, tcbs)
+	end
 end
 
 local function freesection(tid, train, ts_id, ts)