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

turn EncodeDNABlock proc into a define #28186

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Kenionatus
Copy link
Contributor

What Does This PR Do

Turns the proc EncodeDNABlock into a define. It is a one liner that improves readability but otherwise eats up performance, especially during startup.

Why It's Good For The Game

Less startup lag.

Testing

Spawned in as geneticist. Followed the guide on the wiki to make a humanized monkey. Changed the humanized monkey's gender via setting UI block 37. Irradiated SE block 1 until above DAC. Made single block injector and injected myself. Got morphing power. Morphing power worked.
Spawned in as clown. Shot a laser gun and clumsily shot myself in the foot on first shot. Removed clumsy gene via player panel. Shot rest of laser gun charge. Did not shoot myself in the foot.


Declaration

  • I confirm that I either do not require pre-approval for this PR, or I have obtained such approval and have included a screenshot to demonstrate this below.

Changelog

NPFC

@ParadiseSS13-Bot ParadiseSS13-Bot added the -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally label Jan 30, 2025
@Drsmail
Copy link
Contributor

Drsmail commented Jan 30, 2025

Did you get any performance boost? Can you mesure it?

Copy link
Contributor

@Contrabang Contrabang left a comment

Choose a reason for hiding this comment

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

By making this a define, we should also change the capitalization of it

code/__DEFINES/dna.dm Outdated Show resolved Hide resolved
This is to reflect it now being a define.
@Kenionatus
Copy link
Contributor Author

Not tested yet. I pressed the synch button without thinking.

@Kenionatus
Copy link
Contributor Author

@Drsmail I asked whether I need to benchmark it and to quote AA (the only one who responded): "Why is this not a define. That's player count * 52 proc overhead for no reason"
Generating a Tracy trace looks like a lot of work to set up, so I'm not going to do that for such a relatively minor thing that almost can't have any negative impact.

@Kenionatus
Copy link
Contributor Author

Ok, tested again. (This time I got the awesome power of blindness, but same same otherwise.)

@Kenionatus Kenionatus requested a review from Contrabang January 30, 2025 20:19
@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting review This PR is awaiting review from the review team and removed -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally labels Jan 31, 2025
@Burzah Burzah added the Refactor This PR will clean up the code but have the same ingame outcome label Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Status: Awaiting review This PR is awaiting review from the review team Refactor This PR will clean up the code but have the same ingame outcome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants