Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

roofs using queue instead of recursion #7945

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+32 −9
Open

roofs using queue instead of recursion #7945

Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7c0c923
roofs using queue instead of recursion
uuuuhuuuu Dec 29, 2024
d989003
Merge branch 'master' into roof-refactor
cuberound Dec 29, 2024
850a8e1
Update code/game/objects/structures/roof.dm
cuberound Dec 30, 2024
09c536d
adds suggested changes
uuuuhuuuu Dec 30, 2024
be7be38
hm
uuuuhuuuu Dec 30, 2024
6f9ddbb
lazy nodes need to be in init
uuuuhuuuu Dec 30, 2024
d77ff16
improvements
uuuuhuuuu Feb 3, 2025
5b09b4f
Merge branch 'master' into roof-refactor
cuberound Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions code/game/objects/structures/roof.dm
View file
Open in desktop
cuberound marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
neighbor = locate() in adjacent_loc
if(!neighbor)
neighbor = new(adjacent_loc)
return INITIALIZE_HINT_LATELOAD
return INITIALIZE_HINT_ROUNDSTART
cuberound marked this conversation as resolved.
Show resolved Hide resolved

/obj/structure/roof/LateInitialize() //we use late init to allow for lazy nodes to spawn first on mapload
. = ..()
Expand All @@ -51,9 +51,9 @@
/obj/structure/roof/Destroy(force, ...)
if(linked_master)
linked_master.remove_roof(src)
for(var/icon in GLOB.player_list)
var/mob/mob = icon
mob.client.images -= normal_image
linked_master = null;
for(var/mob/mob as anything in GLOB.player_list)
mob.client?.images -= normal_image
return ..()

/obj/structure/roof/proc/add_default_image(subsystem, mob/mob)
Expand All @@ -65,9 +65,12 @@
return
master.connected_roof += src
linked_master = master
var/list/unprocesed_roofs = list()
for(var/direction in CARDINAL_ALL_DIRS)
for(var/obj/structure/roof/roof in get_step(src,direction))
roof.link_master(master)
if(!roof.linked_master)
unprocesed_roofs += roof
return unprocesed_roofs


/obj/effect/roof_node //used for observing if mob is near the roof
Expand Down Expand Up @@ -96,9 +99,12 @@
return
master.connected_nodes += src
linked_master = master
var/list/nerby_nodes = list()
for(var/direction in CARDINAL_ALL_DIRS)
for(var/obj/effect/roof_node/node in get_step(src,direction))
node.link_master(master)
if(!node.linked_master)
nerby_nodes += node
return nerby_nodes
Comment on lines 99 to +107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This proc is likely moot to exist with suggested changes to connect.



/datum/roof_master_node //maintains one block of roof
Expand All @@ -111,7 +117,7 @@
if(connected_nodes)
for(var/obj/effect/roof_node/roof_node in connected_nodes)
qdel(roof_node)
if(connected_nodes)
if(connected_roof)
for(var/obj/structure/roof/roof in connected_roof)
qdel(roof)
return ..()
Expand Down Expand Up @@ -154,10 +160,27 @@
remove_under_roof(living)

/datum/roof_master_node/proc/connect(location)
var/list/unprocesed_nodes = list()
for(var/obj/effect/roof_node/node in location)
node.link_master(src)
unprocesed_nodes += node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to just figure out all the work that needs to be performed, and have that in a list. Likely better to add using |= so you don't add duplicates; but you may want to test what performs better w/ large use cases: as in is byond faster at checking for duplicates than just enqueueing them anyways and finding out we don't need to work on them when you come across a node already set up.



while(length(unprocesed_nodes))
var/obj/effect/roof_node/node = popleft(unprocesed_nodes)
var/list/new_nodes = node.link_master(src)
if(new_nodes)
unprocesed_nodes += new_nodes

var/list/unprocesed_roofs = list()

for(var/obj/structure/roof/roof in location)
roof.link_master(src)
unprocesed_roofs += roof
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to just figure out all the work that needs to be performed, and have that in a list. Likely better to add using |= so you don't add duplicates; but you may want to test what performs better w/ large use cases: as in is byond faster at checking for duplicates than just enqueueing them anyways and finding out we don't need to work on them when you come across a node already set up.


while(length(unprocesed_roofs))
var/obj/structure/roof/roof = popleft(unprocesed_roofs)
var/list/new_roofs = roof.link_master(src)
if(new_roofs)
unprocesed_roofs += new_roofs

/datum/roof_master_node/proc/remove_roof(obj/structure/roof/roof) //roof tile got removed
connected_roof -= roof
Expand Down
Loading