Vending product asset transfer tweaks and testing #68
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-merge-queue bot
pushed a commit
to cmss13-devs/cmss13
that referenced
this pull request
Dec 1, 2023
# About the pull request This PR cherry picks cmss13-devs/cmss13-pve#68 but removes the extra logging. That PR was attempting to resolve server hitching, but the hitching seems to have gone away with it and a TGS update. I am not currently able to discern any noticeable differences from this change, however the change looks correct for the continue check to ever work correctly. See Testing Notes below. Let me know if you do find a change this does improve, or maybe somewhere else we should expect superfluous calls to getFlatIcon. # Explain why it's good for the game The continue check as written looks like it would never work, whereas now the string that will be used for inserting into the list sprites is the same string we check before creating new icons. # Testing Photographs and Procedure <details> <summary>Testing Notes</summary> Debugging only: Lobby (LV-624): One event of registering (master -> init Assets -> vending_products) 55 getFlatIcon calls (.124 total cpu) Game Start: No changes After opening CO vendors and cig vendor: No changes Alt+click self: 77 getFlatIcon calls (.353 total cpu) Alt+click self again: No changes ----- After continue change: Lobby (LV-624): One event of registering (master -> init Assets -> vending_products) 55 getFlatIcon calls (.117 total cpu) Game Start: No changes After opening CO vendors and cig vendor: No changes Alt+click self: 77 getFlatIcon calls (.346 total cpu) Alt+click self again: No changes ------ Debugging only w/ SVN: Lobby (LV-624): One event of registering (master -> init Assets -> vending_products) 55 getFlatIcon calls (.124 total cpu) Game Start: No changes After opening CO vendors and cig vendor: a GET was performed for first CO vendor and first cig vendor opened (never again) Alt+click self: 77 getFlatIcon calls (.351 total cpu) A GET is performed for every item listed Alt+click self again: A GET is performed for every item listed ----- After continue change w/ SVN: Lobby (LV-624): One event of registering (master -> init Assets -> vending_products) 55 getFlatIcon calls (.120 total cpu) Game Start: No changes After opening CO vendors and cig vendor: a GET was performed for first CO vendor and first cig vendor opened (never again) Alt+click self: 77 getFlatIcon calls (.347 total cpu) A GET is performed for every item listed Alt+click self again: A GET is performed for every item listed </details> # Changelog :cl: Morrow code: Corrected a check to avoid repeat work in /datum/asset/spritesheet/vending_products/register() /:cl: --------- Co-authored-by: Morrow <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I suspect something is re-registering vending products. Not sure though.
Secondly, while looking into this I believe I found the check for an already existing sprite on line 323 of asset_list_items.dm is actually incorrect and should be the imgid rather than the icon. I think...