Traps cleanup #1009
Closed
Traps cleanup #1009
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
Testbits isn't necessary here; it's sufficient to use & to see whether the branch has the dangerous_end flag. This is at least as easy to read and removes an unnecessary equality test. Note that this is what do_trap_effects already does in its version of this logic.
The function do_trap_effects duplicates logic from is_valid_shaft_effect_level. The latter function is apparently never called. I suspect the intention was for do_trap_effects to call is_valid_shaft_effect_level and the author forgot to make the change. In any case, this commit removes the logic from do_trap_effects and inserts a call to is_valid_shaft_effect_level.
This makes the comment a little less likely to go out of date if someone starts moving things around in the future.
The current flow is to decide whether the player is trapped when revealing a tile (player_view_update_at calls roll_trap_effects); if so, you.trapped is set to true. The next time the world reacts, do_trap_effects is called and then you.trapped is cleared back to false. Currently do_trap_effects rolls for which trap effect will happen and then checks to see if it is allowed to happen in that place; if not, nothing happens. This has the result that, all else equal, the player is less likely to suffer any trap effect on floors where one of the effect types is ruled out, e.g., the player is less likely to get trapped on Snake:3 than Snake:2. This commit changes do_trap_effects to first find the valid trap effects for the location and then choose among them. So, all else equal, the player will be just as likely to get trapped on a floor where shafts cannot be placed---the other trap effect types will pick up the slack. (In general all else is not equal, since trap effect rate depends on depth; see trap_rate_for_place. But probably this means that code affecting rate of triggering trap effect should be there, and not rolled in with code choosing the effect in do_trap_effects.)
ufshaikh
commented
Mar 12, 2019
ebering
requested changes
Mar 15, 2019
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.
These are few commits cleaning up some minor things I noticed while digging around in traps.cc.
The last commit doesn't seem like an especially elegant way to do this to me---I'm open to suggestions.