Clean up the priming subroutine for explosives#12452
Open
LatoDiafol wants to merge 3 commits into
Open
Conversation
This file contains hidden or 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
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.
About the pull request
Made the prime() function in explosive.dm more streamlined and robust, this involved:
The removal of the "i" variable and replacing it with that simply counted up as the containers were iterated across the first time and counted down when the second loop then transferred the contents of those containers to the "meta-container" that is used to calculate the actual reaction. This was replaced by a call to find the length of the container array instead of counting up in the first loop, this makes the code more robust by keeping each loop independent while also making the code easier to read and comprehend.
Introduction of an error message should an invalid explosive with 0 or less containers that somehow has reagents contained within, theoretically this code should never fire, but it should catch any edge cases I've missed and aid in debugging.
The change was tested using all of the partticular test cases I could think of, the results of which can be found below.
Explain why it's good for the game
Removes fragility from the procedure which could cause it to break in unexpected ways if modified. Removes an ambiguous variable "i" and replaces it with a usefully named variable "container_count"
Testing Photographs and Procedure
Test Cases:
No containers: Properly makes the dud sound and does not trigger volatiles.
One container of an explosive substance: Sets the volatiles flag properly and detonates. (expected 60 units welding fuel present at the point of explosion and the "Trigger Volatiles" flag set as true, Test successful)
Two containers of explosives: Properly combines both canisters and triggers the detonation as it should. (Two containers of 60 units welding fuel each, saw 120 units of welding fuel at the point of explosion and the Trigger Volatiles flag set as expected.)
One container of non explosives: Produces a whiff of steam as expected, but no actual explosion (Explosive contained 60 units of Watermelon Juice)
One Container of Explosives and one container of non explosives (One bottle of 60u Welding Fuel, One bottle of 60u Watermelon Juice, explosion triggered with identical stats to the grenade containing just 60u welding fuel as expected)
Screenshots & Videos
Not Applicable
Changelog
🆑
code: Streamlined the prime() function for explosives in explosives.dm
/:cl: