Viewing Issue Simple Details Jump to Notes ] Wiki ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0008261 [DCSS] Patches crash have not tried 2014-03-11 14:36 2016-09-19 16:48
Reporter johnnyzero View Status public  
Assigned To sgrunt
Priority normal Resolution done  
Status closed   Product Branch 0.14 ancient branch
Summary 0008261: Stepping on corrupted sealed door location causes crash
Description Used corrupt to get past a sealed door. Stepping on the location where the sealed door was located before corruption caused a crash. From the crashlog, it looks like the location is still marked as being a sealed door.
Standing on/in/over feature: sealed door


Crash log: http://dobrazupa.org/morgue/johnnyzero/crash-johnnyzero-20140311-132303.txt [^]
Save: https://dobrazupa.org/saves/johnnyzero-crawl-git-29e27d4c46-140311-1327.tar.bz2 [^]
footv: !lm johnnyzero crash 14 -tv
Additional Information
Tags No tags attached.
Attached Files ? file icon 8261_patch_3.patch [^] (4,665 bytes) 2014-04-04 16:19 [Show Content]

- Relationships

-  Notes
(0025638)
blackcustard (reporter)
2014-03-16 19:48
edited on: 2014-04-03 16:18

Here's a patch.

johnnyzero had it backwards. The sealed door moved, and the square it was on was updated perfectly.

The problem was the door moved onto the player.

(git am < 8261_patch_1.patch)

EDIT: git am 8261_patch_2.patch now. I think.

(0025776)
Reaver (developer)
2014-04-03 21:45
edited on: 2014-04-03 21:54

Removed the bad original patch.

Also, your first commit correcting" the comment looks wrong to me. I believe the comment is referencing this bit of code:

     bool preserve_feat = true;
     dungeon_feature_type feat = DNGN_UNSEEN;

which sets arguments that are later passed to dungeon_terrain_changed() call.

Since this confused you that section could use a clean up anyway - possibly adding a linebreak between the feat =DNGN_UNSEEN line and the altar handling code, but it needs to be careful to describe how the system actually works.

(0025780)
blackcustard (reporter)
2014-04-04 16:19
edited on: 2014-04-05 15:59

Thank you for removing the first patch.

----

I know that preserve_feat is passed to dungeon_terrain_changed. I do not believe that I am confused, but I think you might be.

_corrupt_level_features calls _corrupt_square only if _is_grid_corruptible tells it to. _is_grid_corruptible returns false for stairs/portals/branch entries.

So preserve_feat really doesn't have anything to do with stairs/portals/branch entries.

This behavior has not changed. _is_grid_corruptible has been returning false (EDIT: for stairs/portals/branch entries (forgot to specify this)) since corruption was first added back in e9e1be78 in 2007. Originally, dungeon_terrain_change was given "true" as its fourth argument (preserve_features). The incorrect comment was introduced along with preserve_feat in 90b143eb in 2008, when the function was modified to pass false for altars. Presumably because its committer misunderstood the code.

----

Also note: dungeon_terrain_changed calls _dgn_shift_feature only if preserve_features is true. But _dgn_shift_feature doesn't actually try to shift the feature unless either is_critical_feature tells it to, or the square the feature is in contains a map marker. is_critical_feature cares about altars (for which preserve_feat is false, so _dgn_shift_feature never gets called), stairs/branch entries/portals (for which _corrupt_square is never called), and malign gateways.

So the only things we ACTUALLY try to preserve are malign gateways and things that happen to have map markers. MAT_CORRUPTION_NEXUS is a marker, but it only appears on DNGN_FLOOR.

So really we don't preserve much at all.

----

You were right about one thing: this code was obviously confusing. I have updated the comment, updated the commit messages, and renamed preserve_feat to match preserve_features inside dungeon_change_terrain.

(0025781)
Reaver (developer)
2014-04-05 01:25

I don't really have a response to that. I'm not sure which of us is correct, might be good to get somebody familiar with the code to look at this.

I deleted your 2nd patch, since the 3rd patch supersedes it.
(0025808)
sgrunt (administrator)
2014-04-09 17:47

Patch applied. Thanks!

- Issue History
Date Modified Username Field Change
2014-03-11 14:36 johnnyzero New Issue
2014-03-16 19:48 blackcustard Note Added: 0025638
2014-03-16 19:48 blackcustard File Added: 8261_patch_1.patch
2014-03-16 19:49 blackcustard Note Edited: 0025638
2014-04-03 16:16 blackcustard File Added: 8261_patch_2.patch
2014-04-03 16:18 MarvinPA Category Bug Report => Patches
2014-04-03 16:18 blackcustard Note Edited: 0025638
2014-04-03 21:45 Reaver File Deleted: 8261_patch_1.patch
2014-04-03 21:45 Reaver Note Added: 0025776
2014-04-03 21:45 Reaver Note Added: 0025777
2014-04-03 21:50 Reaver Note Deleted: 0025777
2014-04-03 21:51 Reaver Note Edited: 0025776
2014-04-03 21:53 Reaver Note Edited: 0025776
2014-04-03 21:54 Reaver Note Edited: 0025776
2014-04-04 16:19 blackcustard Note Added: 0025780
2014-04-04 16:19 blackcustard File Added: 8261_patch_3.patch
2014-04-05 01:25 Reaver Note Added: 0025781
2014-04-05 01:25 Reaver File Deleted: 8261_patch_2.patch
2014-04-05 15:59 blackcustard Note Edited: 0025780
2014-04-09 17:47 sgrunt Note Added: 0025808
2014-04-09 17:47 sgrunt Assigned To => sgrunt
2014-04-09 17:47 sgrunt Status new => resolved
2014-04-09 17:47 sgrunt Resolution open => done
2014-04-09 17:47 sgrunt Fixed in Branch => 0.14 development branch
2016-09-19 16:47 johnnyzero Status resolved => closed


Mantis 1.1.8[^]
Copyright © 2000 - 2009 Mantis Group
Powered by Mantis Bugtracker