Skip to content

Commit

Permalink
Fixes catwalk rendering layering and catwalk pipe cap issues (tgstati…
Browse files Browse the repository at this point in the history
…on#85236)

## About The Pull Request

Sooooooo this one's a mess.
First off, layering issues. Pipes, cables, and disposals currently
render over catwalks!

![image](https://github.com/user-attachments/assets/bfb6fc15-878c-4686-aace-57f0b9c6923a)
That's not great.

Looking into it, it seems we've moved the `CATWALK_LAYER` below the
`ABOVE_OPEN_TURF_LAYER`, where the catwalk layer is used for closed
catwalks.

https://github.com/tgstation/tgstation/blob/33e983ced1ac27143c4b87d8761277eea35a6d2a/code/__DEFINES/layers.dm#L148-L150
Which, well, we've *also* made it so all `undertile` stuff gets rendered
at `ABOVE_OPEN_TURF_LAYER` when below a tile.
So! Naively! We swap those around! Easy peasy lemon squeezy.
```dm
 #define ABOVE_OPEN_TURF_LAYER (12 + TOPDOWN_LAYER) 
 #define CATWALK_LAYER (13 + TOPDOWN_LAYER) 
```
And hey! Well!

![image](https://github.com/user-attachments/assets/42d7a8f3-5c17-4039-a76b-dbd789432156)
It's progress!
But as we can see in the bottom right catwalk tile, something's not
rendering right when they're below the tile...

Well, time to take a closer look at our `undertile` element... aaaaaand
would you look at that:

https://github.com/tgstation/tgstation/blob/74f9a4314138afcb04af3cfb452ff167105f022c/code/datums/elements/undertile.dm#L45-L48
We're setting EVERYTHING to the `FLOOR_PLANE` at
`ABOVE_OPEN_TURF_LAYER`, even the stuff that was already on
`FLOOR_PLANE` with its own layer like disposals or cables.
Meaning, layering fuckery ensues, we can't see shit.

So? We just make it only do that when we're not already on the floor
plane.
```dm
	if(PLANE_TO_TRUE(source.plane) != FLOOR_PLANE)
		SET_PLANE_IMPLICIT(source, FLOOR_PLANE)
		source.layer = ABOVE_OPEN_TURF_LAYER
```
This solves it!

![image](https://github.com/user-attachments/assets/f930bd66-87eb-433e-8bf5-09706316ace4)
Progress!

![image](https://github.com/user-attachments/assets/f2f246a2-8524-4186-9ac3-07ac7dcf4288)
...Kind of.
The _layering_ is solved, but unscrewing and rescrewing them seems to
cause pipe caps to manifest out of nowhere!
This _sucks_ for debugging, y'know?
Anyhow, this is based on two different things: an order of operations
issue and catwalks just not being accounted for.

First off! Order of operations.
On `Initialize(...)`, the base `/obj/machinery/atmospherics` registers a
proc that updates pipe caps on `COMSIG_OBJ_HIDE`:

https://github.com/tgstation/tgstation/blob/33e983ced1ac27143c4b87d8761277eea35a6d2a/code/modules/atmospherics/machinery/atmosmachinery.dm#L114-L115
Meanwhile, `/obj/machinery/atmospherics/pipe`, adds the `undertile`
element on its `Initialize(...)`... AFTER calling the parent.

https://github.com/tgstation/tgstation/blob/33e983ced1ac27143c4b87d8761277eea35a6d2a/code/modules/atmospherics/machinery/pipes/pipes.dm#L31-L35
...Which then registers its own proc on `COMSIG_OBJ_HIDE`...

https://github.com/tgstation/tgstation/blob/74f9a4314138afcb04af3cfb452ff167105f022c/code/datums/elements/undertile.dm#L26
This meant that, well, the proc that generates the caps was being called
*before* undertile had a chance to chance to remove the
`TRAIT_UNDERFLOOR` trait... which pipe caps use to work out when to
generate.

https://github.com/tgstation/tgstation/blob/33e983ced1ac27143c4b87d8761277eea35a6d2a/code/modules/atmospherics/machinery/atmosmachinery.dm#L650-L652
So, we swap this around by moving both to a `setup_hiding()` proc which
allows the pipe to register its behaviours before calling the parent and
it register its, without needing to register it before calling the
parent `Initialize(...)`. Cause that's ugly.
Now we're generating pipe caps on catwalks!

But! That brings us perfectly to the next bit. Cause those pipe caps,
even if shown when the tile is open, look *ugly*.
Why, when we open a catwalk, are we having our pipes suddenly extend
onto the neighbouring tiles and catwalks and going down into them from
the top? Arguably, these should behave like they're below tiles, because
they logically are even if not technically so.
Well, actually, we already have a similar situation with bare plating.
It's not applying `TRAIT_UNDERFLOOR`, but also the pipe caps shouldn't
be behaving like they're above a tile, because that'd be ugly- and
that's what it does!

https://github.com/tgstation/tgstation/blob/33e983ced1ac27143c4b87d8761277eea35a6d2a/code/modules/atmospherics/machinery/atmosmachinery.dm#L654-L655
So, we just apply a second check there, `iscatwalkturf(...)`
```dm
	var/turf/node_turf = get_turf(node)
	if(isplatingturf(node_turf) || iscatwalkturf(node_turf))
		continue
```

And? Well!

![image](https://github.com/user-attachments/assets/f297136e-f62e-452b-b711-2f3b69462859)

![image](https://github.com/user-attachments/assets/a3f2df8b-3e22-4474-af32-7e858382f63f)

![image](https://github.com/user-attachments/assets/347a2c3b-8302-47b8-a376-41228fbe537a)
There we go! There's no weird layering, there's no pipe caps where there
shouldn't be, pipes behave like those on catwalks are actually under a
tile.
It looks _clean_.
...

Well, for however clean we can get it to be without making sprites for
the opened catwalks but without the integrated plating so we can make an
overlay and have the pipes/cables/disposals not spontaneously go over
the edges of the catwalk when opened. Or making the under sprites only
have the attachment points in the corners, so it looks like the
pipes/cables/disposals are going over plating instead of what looks like
a raised edge.
## Why It's Good For The Game

Fixes tgstation#84789.
Fixes tgstation#82622.
Screwing open a catwalk suddenly generating pipecaps on neighbouring
closed catwalks and tiles with pipes under them looks weird.
## Changelog
:cl:
fix: Fixed pipes/cables/disposals rendering above closed catwalks.
fix: Fixed catwalks covering pipes generating illogical pipe caps when
screwed.
fix: Opened catwalks are no longer assumed to be above-floor for the
sake of generating pipe caps.
/:cl:
  • Loading branch information
00-Steven authored Aug 14, 2024
1 parent c5e2321 commit 2ce8063
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 13 deletions.
3 changes: 3 additions & 0 deletions code/__DEFINES/dcs/signals/signals_object.dm
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,9 @@
/// from /datum/component/dart_insert/on_reskin()
#define COMSIG_DART_INSERT_PARENT_RESKINNED "dart_insert_parent_reskinned"

/// from /datum/element/undertile/hide()
#define COMSIG_UNDERTILE_UPDATED "undertile_updated"

/// Sent from /obj/item/update_weight_class(). (old_w_class, new_w_class)
#define COMSIG_ITEM_WEIGHT_CLASS_CHANGED "item_weight_class_changed"
/// Sent from /obj/item/update_weight_class(), to its loc. (obj/item/changed_item, old_w_class, new_w_class)
Expand Down
2 changes: 2 additions & 0 deletions code/__DEFINES/is_helpers.dm
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ GLOBAL_LIST_INIT(turfs_openspace, typecacheof(list(

#define isplatingturf(A) (istype(A, /turf/open/floor/plating))

#define iscatwalkturf(A) (istype(A, /turf/open/floor/catwalk_floor))

#define isasteroidturf(A) (istype(A, /turf/open/misc/asteroid))

#define istransparentturf(A) (HAS_TRAIT(A, TURF_Z_TRANSPARENT_TRAIT))
Expand Down
4 changes: 2 additions & 2 deletions code/__DEFINES/layers.dm
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@
#define WIRE_LAYER (9 + TOPDOWN_LAYER)
#define GLASS_FLOOR_LAYER (10 + TOPDOWN_LAYER)
#define TRAM_RAIL_LAYER (11 + TOPDOWN_LAYER)
#define ABOVE_OPEN_TURF_LAYER (12 + TOPDOWN_LAYER)
///catwalk overlay of /turf/open/floor/plating/catwalk_floor
#define CATWALK_LAYER (12 + TOPDOWN_LAYER)
#define ABOVE_OPEN_TURF_LAYER (13 + TOPDOWN_LAYER)
#define CATWALK_LAYER (13 + TOPDOWN_LAYER)

//GAME_PLANE layers
#define BELOW_CLOSED_TURF_LAYER 2.053
Expand Down
11 changes: 9 additions & 2 deletions code/datums/elements/undertile.dm
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@
var/turf/T = get_turf(source)

if(underfloor_accessibility < UNDERFLOOR_INTERACTABLE)
SET_PLANE_IMPLICIT(source, FLOOR_PLANE) // We do this so that turfs that allow you to see what's underneath them don't have to be on the game plane (which causes ambient occlusion weirdness)
source.layer = ABOVE_OPEN_TURF_LAYER
// We only want to change the layer/plane for things that aren't already on the floor plane,
// as overriding the settings for those would cause layering issues
if(PLANE_TO_TRUE(source.plane) != FLOOR_PLANE)
// We do this so that turfs that allow you to see what's underneath them don't have to be on the game plane (which causes ambient occlusion weirdness)
SET_PLANE_IMPLICIT(source, FLOOR_PLANE)
source.layer = ABOVE_OPEN_TURF_LAYER

ADD_TRAIT(source, TRAIT_UNDERFLOOR, REF(src))

if(tile_overlay)
Expand Down Expand Up @@ -77,6 +82,8 @@
if(use_anchor)
source.set_anchored(FALSE)

SEND_SIGNAL(source, COMSIG_UNDERTILE_UPDATED)

/datum/element/undertile/Detach(atom/movable/source, visibility_trait, invisibility_level = INVISIBILITY_MAXIMUM)
. = ..()

Expand Down
18 changes: 14 additions & 4 deletions code/modules/atmospherics/machinery/atmosmachinery.dm
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
turf_loc.add_blueprints_preround(src)

if(hide)
RegisterSignal(src, COMSIG_OBJ_HIDE, PROC_REF(on_hide))
setup_hiding()

SSspatial_grid.add_grid_awareness(src, SPATIAL_GRID_CONTENTS_TYPE_ATMOS)
SSspatial_grid.add_grid_membership(src, turf_loc, SPATIAL_GRID_CONTENTS_TYPE_ATMOS)
Expand All @@ -133,9 +133,18 @@
return ..()

/**
* Handler for `COMSIG_OBJ_HIDE`, connects only if `hide` is set to `TRUE`. Calls `update_cap_visuals` on pipe and its connected nodes
* Sets up our pipe hiding logic, consolidated in one place so subtypes may override it.
* This lets subtypes implement their own hiding logic without needing to worry about conflicts with the parent hiding logic.
*/
/obj/machinery/atmospherics/proc/on_hide(datum/source, underfloor_accessibility)
/obj/machinery/atmospherics/proc/setup_hiding()
// Register pipe cap updating when hidden/unhidden
RegisterSignal(src, COMSIG_OBJ_HIDE, PROC_REF(on_hide))

/**
* Signal handler. Updates both our pipe cap visuals and those of adjacent nodes.
* We update adjacent nodes as their pipe caps are based partially on our state, so they need updating as well.
*/
/obj/machinery/atmospherics/proc/on_hide(datum/source)
SHOULD_CALL_PARENT(TRUE)
SIGNAL_HANDLER

Expand Down Expand Up @@ -651,7 +660,8 @@
if(HAS_TRAIT(node, TRAIT_UNDERFLOOR))
continue

if(isplatingturf(get_turf(node)))
var/turf/node_turf = get_turf(node)
if(isplatingturf(node_turf) || iscatwalkturf(node_turf))
continue

var/connected_dir = get_dir(src, node)
Expand Down
9 changes: 4 additions & 5 deletions code/modules/atmospherics/machinery/pipes/pipes.dm
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@
volume = 35 * device_type
. = ..()

///I have no idea why there's a new and at this point I'm too afraid to ask
/obj/machinery/atmospherics/pipe/Initialize(mapload)
. = ..()
/obj/machinery/atmospherics/pipe/setup_hiding()
AddElement(/datum/element/undertile, TRAIT_T_RAY_VISIBLE) //if changing this, change the subtypes RemoveElements too, because thats how bespoke works

if(hide)
AddElement(/datum/element/undertile, TRAIT_T_RAY_VISIBLE) //if changing this, change the subtypes RemoveElements too, because thats how bespoke works
// Registering on `COMSIG_OBJ_HIDE` would cause order of operations issues with undertile, so we register to run when undertile updates instead
RegisterSignal(src, COMSIG_UNDERTILE_UPDATED, PROC_REF(on_hide))

/obj/machinery/atmospherics/pipe/on_deconstruction(disassembled)
//we delete the parent here so it initializes air_temporary for us. See /datum/pipeline/Destroy() which calls temporarily_store_air()
Expand Down

0 comments on commit 2ce8063

Please sign in to comment.