Implementing the restructure changes

[PowerWyrm]

This week I decided to start implementing all the changes from the restructure to my variant. At this point, there are 35 pages of commits to browse, analyze and implement... and it took me already three days to tackle the first five commits. Needless to say it's gonna take ages...

Anyway, I'm going to post here all the oddities I find along the way. Please tell me if they have been fixed in the latest version of the code.

Commit f2cf3e5 (Implement trap layer):

* the minimum level of some traps has been changed: stat draining darts were minlevel 6, now 2; confusing gas traps were minlevel 1, now also 2; sounds like a copy/paste error

* rubble was not "BORING", it should probably be "INTERESTING"

* square_destroy_trap(): passing "c" as cave parameter, but using the static variable "cave" instead (see also: square_trap_specific)

* cave illumination/darkening has been completely broken when introducing the "INTERESTING" flag; before the restructure, there were two symmetrical bits of code, "feat > FEAT_INVIS" and "square_isinteresting"; this commit broke the symmetry by changing square_isinteresting() to features implementing the INTERESTING flag, which are completely different; this is particularly obvious for a variant that has other floor tiles (dirt, grass, water...) and non-floor tiles (trees, mountains...), which with the new code are memorized/forgotten incorrectly when lit/unlit; IMO there is a confusion between "interesting" and "normal" features here -- the "interesting" flag should be used for targeting code only (there's also a "square_isboring" function used there with a negation, which is completely pointless, "square_isinteresting" should be used instead), and the "normal" flag should be used everywhere else (for lighting/unlighting purposes)

In the code:
wiz_light(): uses "normal" flag (correct)
cave_illuminate(): uses "normal" flag (correct)
project_f(), GF_DARK effect: uses "interesting" flag (incorrect, should use "normal" flag)
map_area(): uses "interesting" flag (incorrect, should use "normal" flag)
cave_unlight(): uses "interesting" flag (incorrect, should use "normal" flag)
target_set_interactive_accept(): uses "interesting" flag (correct, but use "square_isinteresting" instead of the useless "square_isboring" function)

[Derakon]

Rather than going commit-by-commit and manually rebasing your variant off of Vanilla, you might have better luck looking at the diff of restruct Vanilla vs. pre-restruct Vanilla. That would save you from having to implement each bug and then each bug fix as separate tasks.

[Nick]

Rather than going commit-by-commit and manually rebasing your variant off of Vanilla, you might have better luck looking at the diff of restruct Vanilla vs. pre-restruct Vanilla. That would save you from having to implement each bug and then each bug fix as separate tasks.

Two issues with that:

1. The diff will be so large (with so many filename changes) that it is unlikely to be useful
2. PowerWyrm is our most powerful debugging tool - please don't interrupt him while he's operating

PowerWyrm: Thanks for those. The bug tracker now has a milestone for 4.0 which we are using if you want to, but reporting in this thread is also good, it's up to you.

Am I correct in assuming that these errors are still in place in the current state of the code? Some things have been revisited later.

[AnonymousHero]

Two issues with that:

1. The diff will be so large (with so many filename changes) that it is unlikely to be useful
2. PowerWyrm is our most powerful debugging tool - please don't interrupt him while he's operating

Heh. I think it should be noted that, as long as the files remained mostly the same, git/GitHub will actually detect renamed+modified files, so it might be okay to just do a full diff. Then again, it might not.

[Nick]

On the question of INTERESTING vs NORMAL - do you think that either FLOOR or (less likely) EASY is capturing what you are after with NORMAL? I mean this regardless of where they're used incorrectly.

[PowerWyrm]

Rather than going commit-by-commit and manually rebasing your variant off of Vanilla, you might have better luck looking at the diff of restruct Vanilla vs. pre-restruct Vanilla. That would save you from having to implement each bug and then each bug fix as separate tasks.

This is plain impossible because my variant:
- is multiplayer (V assumes in many places that there is one player and one dungeon level at any time)
- is split between client and server (the display for each player + the game engine)
- is real-time (network + time passing code)

Each change (almost each line of code) has to be fully analyzed, because it could impact players, game core, client, network...

[PowerWyrm]

Am I correct in assuming that these errors are still in place in the current state of the code? Some things have been revisited later.

Min level for traps didn't change in latest version, so that's probably a copy/paste error.

Rubble is still not "interesting", no idea if that was intended (before you could examine rubble with the look command, now you can't).

square_destroy_trap() and square_trap_specific() have indeed been fixed in the current code.

Cave illumination/darkening is still broken (non symmetrical): wiz_light() and cave_illuminate() use the "FLOOR" flag (TF_FLOOR), while map_area() and cave_unlight() use the "INTERESTING" flag. IMO the "INTERESTING" flag should only be used for targeting (and look).

[PowerWyrm]

On the question of INTERESTING vs NORMAL - do you think that either FLOOR or (less likely) EASY is capturing what you are after with NORMAL? I mean this regardless of where they're used incorrectly.

In V, it's indeed the FLOOR flag that must be used. Clearly:
- remove square_isboring() and use square_isinteresting() in the targeting code
- use square_isfloor() instead of square_isinteresting() in illumination/darkening code

In variants such as PWMAngband that has a wilderness, it's not so simple. There need to be another flag for illumination/darkening code, which has to be set on floor squares, as well as non-floor squares (water, grass, ...).

[PowerWyrm]

Trap layer and new SQUARE flags is now implemented... and I must have goofed somewhere, because when I try to cast a spell with a mage, I sometimes get "you cannot see" messages while standing in a lit room. I'll have to investigate...

While implementing the trap layer for PWMAngband, I had to port what was done for features concerning client/server preferences (V has only one set of preferences, PWMAngband has two: one for each client, one for the server -- which is for example used while submitting character dumps). While doing that, I found out that trap lighting effects were omitted from the restructure (see parse_prefs_trap), so traps are not lit like before when playing with graphics. Shouldn't be hard to reimplement...

[PowerWyrm]

Trap layer and new SQUARE flags is now implemented... and I must have goofed somewhere, because when I try to cast a spell with a mage, I sometimes get "you cannot see" messages while standing in a lit room. I'll have to investigate...

And I found out why and fixed it... Now back to more testing and more implementing.

[Nick]

Thanks for all that - it will get fixed before too long, but probably not immediately. And I will continue to watch this thread...

[PowerWyrm]

Found a problem about features like doors not "marked" (as per the SQUARE_MARK flag) when using the DM char which generates all levels "lit" while testing my variant, so I donwloaded the latest nightly build to see if the problem was present in that build. Well the problem doesn't exist in V, so I'll have to investigate on my own, but as soon as I tried to use the (L)ook command the game crashed. Just FYI...

[Nick]

Found a problem about features like doors not "marked" (as per the SQUARE_MARK flag) when using the DM char which generates all levels "lit" while testing my variant, so I donwloaded the latest nightly build to see if the problem was present in that build. Well the problem doesn't exist in V, so I'll have to investigate on my own, but as soon as I tried to use the (L)ook command the game crashed. Just FYI...

Thanks - I can't reproduce. Which OS?

[PowerWyrm]

Windows 7. Game crashes as soon as I press L (angband-vTNG-1116-g0a1d754).

[PowerWyrm]

BTW everything in shops cost a fortune now (150 gold for a plain whip, 6 gold for any base supply -- food, ammo, torch), is that intended?