[MIRROR] Refactors tram code to be more robust (#6298)

* Refactors tram code to be more robust (#59596)

Tram code has a lot of locate() in list, a lot of unnecessary typechecking and generally bad practices. This refactors tramcode to be easily more maintainable and to have more consistent behaviour, as well as removing any unnecessary code.

* Refactors tram code to be more robust

Co-authored-by: Watermelon914 <37270891+Watermelon914@users.noreply.github.com>

code/game/machinery/computer/tram_controls.dm

@@ -23,15 +23,13 @@
  * Locates tram parts in the lift global list after everything is done.
  */
 /obj/machinery/computer/tram_controls/proc/find_tram()
-	var/obj/structure/industrial_lift/tram/central/tram_loc = locate() in GLOB.lifts
-	tram_part = tram_loc //possibly setting to something null, that's fine, but
-	tram_part.find_our_location()
+	tram_part = GLOB.central_tram //possibly setting to something null, that's fine
 
 /obj/machinery/computer/tram_controls/ui_state(mob/user)
 	return GLOB.not_incapacitated_state
 
 /obj/machinery/computer/tram_controls/ui_status(mob/user,/datum/tgui/ui)
-	if(tram_part.travelling)
+	if(tram_part?.travelling)
 		return UI_CLOSE
 	if(!in_range(user, src) && !isobserver(user))
 		return UI_CLOSE
@@ -45,8 +43,11 @@
 
 /obj/machinery/computer/tram_controls/ui_data(mob/user)
 	var/list/data = list()
-	data["moving"] = tram_part.travelling
+	data["moving"] = tram_part?.travelling
 	data["broken"] = tram_part ? FALSE : TRUE
+	var/obj/effect/landmark/tram/current_loc = tram_part?.from_where
+	if(current_loc)
+		data["tram_location"] = current_loc.name
 	return data
 
 /obj/machinery/computer/tram_controls/ui_static_data(mob/user)
@@ -63,9 +64,8 @@
  */
 /obj/machinery/computer/tram_controls/proc/get_destinations()
 	. = list()
-	for(var/obj/effect/landmark/tram/destination in GLOB.landmarks_list)
+	for(var/obj/effect/landmark/tram/destination as anything in GLOB.tram_landmarks)
 		var/list/this_destination = list()
-		this_destination["here"] = destination == tram_part.from_where
 		this_destination["name"] = destination.name
 		this_destination["dest_icons"] = destination.tgui_icons
 		this_destination["id"] = destination.destination_id
@@ -75,14 +75,15 @@
 	. = ..()
 	if(. || tram_part.travelling)
 		return
-	var/destination_name = params["destination"]
+	var/destination_id = params["destination"]
 	var/obj/effect/landmark/tram/to_where
-	for(var/obj/effect/landmark/tram/destination in GLOB.landmarks_list)
-		if(destination.name == destination_name)
+	for(var/obj/effect/landmark/tram/destination as anything in GLOB.tram_landmarks)
+		if(destination.destination_id == destination_id)
 			to_where = destination
+			break
 	if(!to_where)
-		CRASH("Controls couldn't find the destination \"[destination_name]\"!")
+		return
 	if(tram_part.controls_locked || tram_part.travelling) // someone else started
 		return
-	tram_part.tram_travel(tram_part.from_where, to_where)
-	update_static_data(usr) //show new location of tram
+	tram_part.tram_travel(to_where)
+	return TRUE

code/game/objects/structures/industrial_lift.dm

@@ -172,6 +172,7 @@ GLOBAL_LIST_EMPTY(lifts)
 	if(!lift_master_datum)
 		lift_master_datum = new(src)
 
+
 /obj/structure/industrial_lift/proc/UncrossedRemoveItemFromLift(datum/source, atom/movable/potential_rider)
 	SIGNAL_HANDLER
 	RemoveItemFromLift(potential_rider)
@@ -411,14 +412,23 @@ GLOBAL_LIST_EMPTY(lifts)
 	var/initial_id = "middle_part"
 	var/obj/effect/landmark/tram/from_where
 	var/travel_direction
-	var/time_inbetween_moves = 1
 
+GLOBAL_DATUM(central_tram, /obj/structure/industrial_lift/tram/central)
+
+/obj/structure/industrial_lift/tram/Initialize(mapload)
+	. = ..()
+	return INITIALIZE_HINT_LATELOAD
 
 /obj/structure/industrial_lift/tram/central//that's a surprise tool that can help us later
 
 /obj/structure/industrial_lift/tram/central/Initialize(mapload)
+	if(GLOB.central_tram)
+		return INITIALIZE_HINT_QDEL
+
 	. = ..()
+
 	SStramprocess.can_fire = TRUE
+	GLOB.central_tram = src
 
 /obj/structure/industrial_lift/tram/LateInitialize()
 	. = ..()
@@ -435,20 +445,10 @@ GLOBAL_LIST_EMPTY(lifts)
  * even in the worst cast scenario.
  */
 /obj/structure/industrial_lift/tram/proc/find_our_location()
-	if(!from_where)
-		for(var/obj/effect/landmark/tram/our_location in GLOB.landmarks_list)
-			if(our_location.destination_id == initial_id)
-				from_where = our_location
-				break
-
-/obj/structure/industrial_lift/tram/central/find_our_location() //the tram knows where it is by knowing where it isn't
-	..()
-	for(var/location in lift_master_datum.lift_platforms)
-		var/obj/structure/industrial_lift/tram/tram_location = location
-		var/turf/turf = get_turf(src)
-		var/where_we_are = locate(/obj/effect/landmark/tram) in turf.contents
-		if(where_we_are)
-			tram_location.from_where = where_we_are //this gets set by the tram movement too but this actually makes sure we're at the dock we were moved to to prevent blender mode
+	for(var/obj/effect/landmark/tram/our_location as anything in GLOB.tram_landmarks)
+		if(our_location.destination_id == initial_id)
+			from_where = our_location
+			break
 
 /obj/structure/industrial_lift/tram/use(mob/user) //dont click the floor dingus we use computers now
 	return
@@ -469,18 +469,21 @@ GLOBAL_LIST_EMPTY(lifts)
  * literally ripping itself apart. The proc handles the first move before the subsystem
  * takes over to keep moving it in process()
  */
-/obj/structure/industrial_lift/tram/proc/tram_travel(obj/effect/landmark/tram/from_where, obj/effect/landmark/tram/to_where)
+/obj/structure/industrial_lift/tram/proc/tram_travel(obj/effect/landmark/tram/to_where)
+	if(to_where == from_where)
+		return
+
 	visible_message("<span class='notice'>[src] has been called to the [to_where]!</span")
 
 	lift_master_datum.set_controls(LOCKED)
+	travel_direction = get_dir(from_where, to_where)
+	travel_distance = get_dist(from_where, to_where)
+	//first movement is immediate
 	for(var/obj/structure/industrial_lift/tram/other_tram_part as anything in lift_master_datum.lift_platforms) //only thing everyone needs to know is the new location.
 		if(other_tram_part.travelling) //wee woo wee woo there was a double action queued. damn multi tile structs
 			return //we don't care to undo locked controls, though, as that will resolve itself
 		other_tram_part.travelling = TRUE
 		other_tram_part.from_where = to_where
-	travel_direction = get_dir(from_where, to_where)
-	travel_distance = get_dist(from_where, to_where)
-	//first movement is immediate
 	lift_master_datum.MoveLiftHorizontal(travel_direction, z)
 	travel_distance--
 
@@ -495,12 +498,12 @@ GLOBAL_LIST_EMPTY(lifts)
  */
 /obj/structure/industrial_lift/tram/proc/unlock_controls()
 	visible_message("<span class='notice'>[src]'s controls are now unlocked.</span")
-	for(var/lift in lift_master_datum.lift_platforms) //only thing everyone needs to know is the new location.
-		var/obj/structure/industrial_lift/tram/other_tram_part = lift
-		other_tram_part.travelling = FALSE
-		other_tram_part.find_our_location()
+	for(var/obj/structure/industrial_lift/tram/tram_part as anything in lift_master_datum.lift_platforms) //only thing everyone needs to know is the new location.
+		tram_part.travelling = FALSE
 		lift_master_datum.set_controls(UNLOCKED)
 
+GLOBAL_LIST_EMPTY(tram_landmarks)
+
 /obj/effect/landmark/tram
 	name = "tram destination" //the tram buttons will mention this.
 	icon_state = "tram"
@@ -508,6 +511,15 @@ GLOBAL_LIST_EMPTY(lifts)
 	///icons for the tgui console to list out for what is at this location
 	var/list/tgui_icons = list()
 
+/obj/effect/landmark/tram/Initialize()
+	. = ..()
+	GLOB.tram_landmarks += src
+
+/obj/effect/landmark/tram/Destroy()
+	GLOB.tram_landmarks -= src
+	return ..()
+
+
 /obj/effect/landmark/tram/left_part
 	name = "West Wing"
 	destination_id = "left_part"

code/modules/assembly/doorcontrol.dm

@@ -221,39 +221,44 @@
 	///for finding the landmark initially - should be the exact same as the landmark's destination id.
 	var/initial_id
 	///this is our destination's landmark, so we only have to find it the first time.
-	var/obj/effect/landmark/tram/to_where
+	var/datum/weakref/to_where
+
+/obj/item/assembly/control/tram/Initialize()
+	. = ..()
+	return INITIALIZE_HINT_LATELOAD
+
+/obj/item/assembly/control/tram/LateInitialize()
+	. = ..()
+	//find where the tram needs to go to (our destination). only needs to happen the first time
+	for(var/obj/effect/landmark/tram/our_destination as anything in GLOB.tram_landmarks)
+		if(our_destination.destination_id == initial_id)
+			to_where = WEAKREF(our_destination)
+			break
+
+/obj/item/assembly/control/tram/Destroy()
+	to_where = null
+	return ..()
 
 /obj/item/assembly/control/tram/activate()
 	if(cooldown)
 		return
 	cooldown = TRUE
 	addtimer(VARSET_CALLBACK(src, cooldown, FALSE), 2 SECONDS)
-	var/obj/structure/industrial_lift/tram/tram_part
-
-	var/obj/machinery/computer/tram_controls/computer = locate(/obj/machinery/computer/tram_controls) in GLOB.machines
-	tram_part = computer?.tram_part
+	var/obj/structure/industrial_lift/tram/tram_part = GLOB.central_tram
 	if(!tram_part)
 		say("The tram is not responding to call signals. Please send a technician to repair the internals of the tram.")
 		return
-	if(!tram_part.from_where) //edge case where the tram has not moved yet and set up it's landmarks but has been called
-		for(var/obj/effect/landmark/tram/tram_landmark in GLOB.landmarks_list)
-			if(tram_landmark.destination_id == tram_part.initial_id)
-				tram_part.from_where = tram_landmark
-				break
-	//find where the tram is going to/is
-	var/obj/effect/landmark/tram/from_where = tram_part.from_where
 	if(tram_part.travelling) //in use
-		say("The tram is already travelling to [from_where].")
+		say("The tram is already travelling to [tram_part.from_where].")
 		return
 	if(!to_where)
-		//find where the tram needs to go to (our destination). only needs to happen the first time
-		for(var/obj/effect/landmark/tram/our_destination in GLOB.landmarks_list)
-			if(our_destination.destination_id == initial_id)
-				to_where = our_destination
-				break
-	if(from_where == to_where) //already here
+		return
+	var/obj/effect/landmark/tram/current_location = to_where.resolve()
+	if(!current_location)
+		return
+	if(tram_part.from_where == current_location) //already here
 		say("The tram is already here. Please board the tram and select a destination.")
 		return
 
-	say("The tram has been called to [to_where]. Please wait for its arrival.")
-	tram_part.tram_travel(from_where, to_where)
+	say("The tram has been called to [current_location.name]. Please wait for its arrival.")
+	tram_part.tram_travel(current_location)

tgui/packages/tgui/interfaces/TramControl.js

@@ -49,36 +49,15 @@ const BrokenTramDimmer = () => {
   );
 };
 
-const MovingTramDimmer = () => {
-  return (
-    <Dimmer>
-      <Stack vertical>
-        <Stack.Item>
-          <Icon
-            ml={10}
-            name="sync-alt"
-            color="green"
-            size={11}
-          />
-        </Stack.Item>
-        <Stack.Item mt={5} fontSize="14px" color="green">
-          The tram is travelling to {current_loc[0].name}!
-        </Stack.Item>
-      </Stack>
-    </Dimmer>
-  );
-};
-
 export const TramControl = (props, context) => {
   const { act, data } = useBackend(context);
   const {
     broken,
     moving,
     destinations,
+    tram_location,
   } = data;
 
-  const current_loc = (destinations ? destinations.filter(
-    dest => dest.here === 1) : null);
   const [
     transitIndex,
     setTransitIndex,
@@ -96,7 +75,7 @@ export const TramControl = (props, context) => {
             />
           </Stack.Item>
           <Stack.Item mt={5} fontSize="14px" color="green">
-            The tram is travelling to {current_loc[0].name}!
+            The tram is travelling to {tram_location}!
           </Stack.Item>
         </Stack>
       </Dimmer>
@@ -105,9 +84,10 @@ export const TramControl = (props, context) => {
   const Destination = props => {
     const { dest } = props;
     const getDestColor = dest => {
-      const here = dest.name === current_loc[0].name;
+      if (!tram_location) return "bad";
+      const here = dest.name === tram_location;
       const selected = transitIndex === destinations.indexOf(dest);
-      return !current_loc ? "bad" : here ? "blue" : selected ? "green" : "transparent";
+      return !tram_location ? "bad" : here ? "blue" : selected ? "green" : "transparent";
     };
     return (
       <Stack vertical>
@@ -185,11 +165,11 @@ export const TramControl = (props, context) => {
               <Stack.Item fontSize="16px" mt={1} mb={9} textAlign="center" grow>
                 <Button
                   disabled={
-                    current_loc[0].name === destinations[transitIndex].name
+                    tram_location === destinations[transitIndex].name
                   }
                   content="Send Tram"
                   onClick={() => act('send', {
-                    destination: destinations[transitIndex].name,
+                    destination: destinations[transitIndex].id,
                   })} />
               </Stack.Item>
             </Stack>