Skip to content
New issue

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

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

Already on GitHub? # to your account

Replay subsystem fixes + optimizations #4771

Merged
merged 13 commits into from
Jan 17, 2025

Conversation

Absolucy
Copy link
Member

@Absolucy Absolucy commented Jan 6, 2025

About The Pull Request

memory usage go brrr

Changelog

🆑
fix: Fixed the replays subsystem still marking objects when disabled.
qol: Optimized the replays subsystem a bit.
/:cl:

@Absolucy Absolucy added the Fix fix da bug label Jan 6, 2025
@Absolucy Absolucy changed the title Fix SSdemo still marking objects when disabled Replay subsystem fixes + optimizations Jan 6, 2025
@Absolucy Absolucy added the Performance / Optimization the number going down makes me happy :3 label Jan 6, 2025
@Absolucy
Copy link
Member Author

Absolucy commented Jan 7, 2025

tested locally and everything seems to work fine

Copy link
Member Author

@Absolucy Absolucy left a comment

Choose a reason for hiding this comment

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

just explaining the reasons behind some of my changes here

@@ -495,4 +537,4 @@ SUBSYSTEM_DEF(demo)
if(marked_dirty[M])
marked_dirty -= M
if(initialized)
del_list["\ref[M]"] = 1
del_list[ref(M)] = TRUE
Copy link
Member Author

Choose a reason for hiding this comment

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

i benchmarked it - lone ref(thing) is faster than lone "\ref[thing]" - but only by itself, \ref[thing] is faster when part of a larger string

Comment on lines +460 to +466
// use for stuff that should appear in the main/targetless chat panel
/datum/controller/subsystem/demo/proc/write_chat_global(text)
if(disabled)
return
var/json_encoded = json_encode(text)
write_event_line("chat global [last_chat_message == json_encoded ? "=" : json_encoded]")
last_chat_message = json_encoded
Copy link
Member Author

@Absolucy Absolucy Jan 7, 2025

Choose a reason for hiding this comment

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

this needs yogstation13/demo-viewer#42 in order to properly show in the viewer, but it won't break viewing replays without that PR, it just won't show the global messages.


// write a "snapshot" of the world at this point.
// start with turfs
log_world("Writing turfs...")
WRITE_LOG_NO_FORMAT(GLOB.demo_log, "init [world.maxx] [world.maxy] [world.maxz]\n")
marked_turfs.Cut()
marked_turfs.len = 0
Copy link
Member Author

Choose a reason for hiding this comment

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

list.len = 0 is slightly faster than list.Cut() according to zewaka and kapu in coderbus

Comment on lines +29 to +30
//var/last_queued = 0
//var/last_completed = 0
Copy link
Member Author

Choose a reason for hiding this comment

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

these vars are never used anywhere anyways at the moment

msg += "}"
return ..()

/datum/controller/subsystem/demo/proc/disable()
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to its own proc to avoid code repetition in case it needs to be disabled outside of init

WRITE_LOG_NO_FORMAT(GLOB.demo_log, "[line]\n")
else
pre_init_lines += line
var/disabled = FALSE
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm adding this var instead of just using can_fire like I did previously so it can be manually paused without disabling it


/client/Del()
/client/Destroy()
Copy link
Member Author

Choose a reason for hiding this comment

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

/client/Del() calls Destroy() anyways

/client/New()
SSdemo.write_event_line("login [ckey]")
. = ..()
return ..()
Copy link
Member Author

Choose a reason for hiding this comment

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

same effect, but better to read imo

Comment on lines +26 to +30
if(isturf(changed_on)) { \
SSdemo.mark_turf(changed_on); \
} else if(isobj(changed_on) || ismob(changed_on)) { \
SSdemo.mark_dirty(changed_on); \
}
Copy link
Member Author

Choose a reason for hiding this comment

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

if it's a turf, then it's not an obj or mob, so changing to else if here should a tiny few cpu cycles

also formatted it better so it's less of an eyesore

@Absolucy Absolucy merged commit 7860ff0 into Monkestation:master Jan 17, 2025
24 checks passed
@Absolucy Absolucy deleted the fix-replay-thing branch January 17, 2025 02:21
github-actions bot added a commit that referenced this pull request Jan 17, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Fix fix da bug Performance / Optimization the number going down makes me happy :3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant