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.

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

Take advantage of how thing.get_all_contents() already includes src as part of the contents list #1871

Merged
merged 2 commits into from
Jun 13, 2024

Conversation

LikeLakers2
Copy link

About The Pull Request

Certain objects in SS13 make checks on both an atom and that atom's contents. For example, /obj/effect/gun_check_blocker/CanPass() checks the atom and all of the atom's contents, preventing the atom from moving through it if any of them are of type /obj/item/gun.

However, when doing this, the code assumes that thing.get_all_contents() does not include thing - and so, performs the check twice - once on thing, and once on every object in thing.get_all_contents().

Yet, /atom/proc/get_all_contents() already includes src as part of the list. In fact, it even initializes the list with src in it:

///Returns the src and all recursive contents as a list.
/atom/proc/get_all_contents(ignore_flag_1)
. = list(src)
var/i = 0
while(i < length(.))
var/atom/checked_atom = .[++i]
if(checked_atom.flags_1 & ignore_flag_1)
continue
. += checked_atom.contents

This PR takes advantage of this fact, removing the redundant check on thing, as thing will just be checked again within the for loop over thing.get_all_contents().

Why It's Good For The Game

Removes redundant code, resulting in a little less CPU time used checking an atom and all its contents.

Changelog

N/A - this does not include any changes that players would notice.

Copy link
Collaborator

@Zonespace27 Zonespace27 left a comment

Choose a reason for hiding this comment

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

Lgtm mostly

monkestation/code/modules/ghost_players/area_changes.dm Outdated Show resolved Hide resolved
@DexeeXI
Copy link

DexeeXI commented Jun 6, 2024

If Zone says a particular change is pretty good, then it should be pretty good to do.

@dwasint dwasint merged commit be26efd into Monkestation:master Jun 13, 2024
20 checks passed
@LikeLakers2 LikeLakers2 deleted the content-checker-optimization branch June 13, 2024 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants