Implementing the restructure changes

[Nick]

Just found a very entertaining bug in my variant: the place_monster() routine checks if there is no monster on the spot where the new monster is to be placed with a call to square_monster()... but doesn't check if there is no player character. This leads to amusing situations where the monster is placed on top of a player character, making that character unseen and unable to move until the corresponding player leaves the game.

This problem may arise if the place_monster() routine is called without a proper check for empty space. In most cases, the caller will make a check (see place_new_monster_group -- check vs square_isempty)... but I'm not sure it's the case everywhere. A double check on the 4.0 source code would be useful...

I think it's all good, but I'm putting an explicit check in place_new_monster_one() to cut off future problems.

[PowerWyrm]

Change 94b1c24:

In list-spell-effects.h, duration for RSF_BLIND was given by RV(11,1,4,0).
In monster_spell.txt, it became dice:11+1d8.

[PowerWyrm]

Change 32aaf38:

In process_monster_grab_objects(), the loop on objects on the floor was checking vs non-gold items to update lore. Since the check has been moved outside of the loop, the flags are now set if there is only a pile of gold on the floor, while they shouldn't.

[PowerWyrm]

Effects for monster/player spells implemented. Now it's time to do the same for spells in PvP and MvM modes...

And 4.0.0 has been released, I'm still 10 months and 29 pages of changesets behind. It's gonna be hard to catch up...

[PowerWyrm]

In make_attack_spell(), the code makes a special case for RSF_HASTE, making effect_handler_MON_TIMED_INC() useless because it's never called.

[PowerWyrm]

Changes 7a23301 + ee69d8c:

It seems that the old "visual" and "drawn" flags are not handled correctly in the new code. In display_bolt(), the "visual" flag has no effect at all, while in the old code, it was used to check for delay while moving along the projection path. In display_explosion(), the "drawn" flag works as intended, but in the old code, both "drawn" and "visual" were used for the delay.

[PowerWyrm]

I'm currently looking at changes in the birth process and found out that get_stats() uses a hardcoded value of 18 to roll stats. This was correct before, but now that charisma is gone, this value should be adjusted. This probably has little impact in the function, but the following should be more correct (and clearer to understand):

	int dice[3 * STAT_MAX];

	/* Roll and verify some stats */
	while (TRUE) {
		/* Roll some dice */
		for (j = i = 0; i < 3 * STAT_MAX; i++) {
			/* Roll the dice */
			dice[i] = randint1(3 + i % 3);

			/* Collect the maximum */
			j += dice[i];
		}

		/* Verify totals */
		if ((j > 7 * STAT_MAX) && (j < 9 * STAT_MAX)) break;
	}

[PowerWyrm]

The arg_graphics and arg_graphics_nice externs are defined twice in the code if you use the Windows port (ui-prefs.h + win-layout.c).

[Nick]

Changes 7a23301 + ee69d8c:

It seems that the old "visual" and "drawn" flags are not handled correctly in the new code. In display_bolt(), the "visual" flag has no effect at all, while in the old code, it was used to check for delay while moving along the projection path. In display_explosion(), the "drawn" flag works as intended, but in the old code, both "drawn" and "visual" were used for the delay.

I've filed this as a bug. I think I'm up to date with the rest, except for the RSF_HASTE silliness, which I'm leaving because the message issues involved are being dealt with.

[PowerWyrm]

Looking at the new monster lore system:

- when parsing the monster file, first base flags are parsed and monster flags are set accordingly, then "on" flags are parsed and added, then "off" flags are parsed and removed from monster flags

- lore flags are saved directly from monster flags

- when parsing the lore file, lore flags are first all set in parse_lore_base(), then parsed and set again in parse_lore_flags(), then combined with base flags in finish_parse_lore()

I don't understand the last part... if flags are saved directly, they should be loaded directly too, so the only useful code here is the one in parse_lore_flags().

[PowerWyrm]

Looking at the new monster lore system:

- when parsing the monster file, first base flags are parsed and monster flags are set accordingly, then "on" flags are parsed and added, then "off" flags are parsed and removed from monster flags

- lore flags are saved directly from monster flags

- when parsing the lore file, lore flags are first all set in parse_lore_base(), then parsed and set again in parse_lore_flags(), then combined with base flags in finish_parse_lore()

I don't understand the last part... if flags are saved directly, they should be loaded directly too, so the only useful code here is the one in parse_lore_flags().

Nevermind... looking more closely, I found out that the "base" part in the lore file is only saved when the lore is full, so it's logical to set all the flags in parse_lore_base().

[PowerWyrm]

Change 4ef6687: in write_monster_entries(), file descriptor "old" is never closed.

[PowerWyrm]

Change 4ef6687: in write_monster_entries(), file descriptor "old" is never closed.

... and power arrays are not freed if new_monster.txt file cannot be created.

[PowerWyrm]

Change 3767674: in remove_bad_spells(), pflags are copied using of_copy() while it clearly should be pf_copy().

[PowerWyrm]

Now this is really puzzling me...

Change 1c405e6:
- the first parameter for the HASTE spell (monsters) is changed from MON_TMD_FAST to FAST
- effect_param() is modified to take into account monster timed effect names, but the function is called after taking into account player timed effect names, which also include "FAST" as a valid name
- the parser parses monster_spell.txt, examines the HASTE spell, calls effect_param() which should return TMD_FAST (0) and not MON_TMD_FAST (5)
- a monster casts HASTE, effect_do() should be called with 0 as p1 parameter instead of 5, which in return should call mon_inc_timed() with 0 as effect which is MON_TMD_SLEEP!
- the monster should be put to sleep

When I launch the latest 4.0 and summon a Berserker, he casts HASTE perfectly normally...