Help me make my new variant! (please!)

[Pete Mack]

Stealth is exponential--an increase of +3 cuts the chance of waking (per game turn) by a factor of two. At stealth 30, the chance of detection is cut by 1/1000 (compares to a H-T warrior). Monsters can still wake up, but it is highly unlikely in the time it takes to cross a room, especially at speed +10 or higher.

[will_asher]

It looks like this is actually a bit of work. The way I would do it is:

1. Add a function (probably in obj-util.c) called something like object_weight(). This should take a struct object * argument, and should take the object's actual weight and then run through all the curses on the object add weight whenever a curse has weight, and return the total.
2. Then search though for all uses of object weight in calculations or display, and replace obj->weight with object_weight(obj).

Alternatively, you could file it as an issue for Vanilla and hope someone else does it...

Done.
Now vanilla can copy me.

/* Modifiers to object weight */
int object_weight(const struct object* obj)
{
	int weight = obj->weight;
	int index = 0;

	/* Check for curses */
	struct curse_data* curse = obj ? obj->curses : NULL;

	/* (copied from calc_bonuses()) */
	while (obj) {
		/* Curses may affect weight (index zero is original object) */
		if ((obj->weight) && (index)) weight += obj->weight;

		/* next curse if any */
		if (curse) {
			index++;
			obj = NULL;
			while (index < z_info->curse_max) {
				if (curse[index].power) {
					obj = curses[index].obj;
					break;
				}
				else {
					index++;
				}
			}
		}
		else {
			obj = NULL;
		}
	}

	return weight;
}

[Pete Mack]

Why do you add obj->weight to itself?

[Pete Mack]

Here's the usual idiom.
Check preconditions. If not met, return early
Think what your iterator is. If it is over an array, usually use a for loop. Figure your termination condition.

You will see this idiom all through the code base. It s standard for a reason. (In this case it's a relational aggregator.)

/* Modifiers to object weight */
int object_weight(const struct object* obj)
{
    int weight = obj->weight;   
    const struct curse* curses = obj-> curses;
    if (!curses) return weight;
    for (int i=0; i < z_info->curse_max && curses[i].power; i++) {
        if ( [[[ curses[i] type is cumbersome ]]])
             weight += [[[ cumbersome curse  constant ]]];
    }
    return weight;
}

[Nick]

Pete, I think you're missing that there is a struct object associated with each curse for easy storage of object properties, so the loop need to run across all the curse objects and add their weight.

So I think the original code is correct.

[Pete Mack]

But if there is an array of curses for each object, surely they all share the same obj pointer?

[Nick]

But if there is an array of curses for each object, surely they all share the same obj pointer?

Each object has a curses array where each entry is a struct of two integers, power and timeout (for timed effects); the object has the curse if the power is non-zero. The actual curses are stored in the global curses[], which is of struct curse - that struct contains a struct object for the actual curse effects, plus name, description, etc.

The reason it's like this is that when I was designing the curse struct, I found myself mimicking the object struct, which makes sense because curses just change the behaviour of an object.

[will_asher]

Doing grenades was easy was DaJAngband, but now the way the code has changed, a lot of things are harder to customize for someone like me who's not fluent in C.

Here's what I'm trying to do:
(in player_attack.c)

 */
static struct attack_result make_ranged_throw(struct player *p,
	struct object *obj, struct loc grid)
{
	char *hit_verb = mem_alloc(20 * sizeof(char));
	struct attack_result result = {false, 0, 0, hit_verb};
	struct monster *mon = square_monster(cave, grid);
	int b = 0, s = 0;

	my_strcpy(hit_verb, "hits", 20);

	/* If we missed then we're done */
	if (!test_hit(chance_of_missile_hit(p, obj, NULL, mon), mon->race->ac))
		return result;

	result.success = true;

	/* EXPLODEing (with grenades) works separately */
	if (of_has(obj->flags, OF_EXPLODE)) {
		result.dmg = ranged_damage(p, mon, obj, NULL, 0, 0);
		/* Use "STRIKE" effect but use "other" param to bypass asking for target and requirement of being in view of the PC. */
		effect_simple(EF_STRIKE, source_player(), format("%d", result.dmg), 0, 2, 9, grid->y, grid->x,
			NULL);
		return result;
	}
...<rest of function>

using "grid->y" and "grid->x" gives me errors. But I can't pass a grid to effect_simple, I need x and y coordinates. How do I access x and y without causing errors here?
Also, the way the code does brands and slays now, I don't know how to get the brand type from the grenade to know what subtype to use with effect_simple here.

[AnonymousHero]

A couple of notes about the code: (I've marked inline with an ascii arrow.)

/* Modifiers to object weight */
int object_weight(const struct object* obj)
{
           <--- probably a good idea to assert that obj != NULL here.
	int weight = obj->weight;  <-- This assumes obj is not null, or Undefined Behavior ensues
	int index = 0;

	/* Check for curses */
	struct curse_data* curse = obj ? obj->curses : NULL; 
        ^-- You don't need to test for "obj" here, it cannot be null by definition because
              you've dereferenced it when initializing "weight" above


	/* (copied from calc_bonuses()) */
	while (obj) {
		/* Curses may affect weight (index zero is original object) */
		if ((obj->weight) && (index)) weight += obj->weight;  
                ^-- You don't need to have "obj->weight" in the condition here, adding 0 doesn't change anything, so you don't need to check
                EDIT: You can also just declare "index = 1" rather than "index = 0" at the start and drop the conditional altogether
                        If index starts at 1 you can just add the weight regardless.


		/* next curse if any */
		if (curse) {
			index++;
			obj = NULL;
			while (index < z_info->curse_max) {
				if (curse[index].power) {
					obj = curses[index].obj;
					break;
				}
				else { 
                                    ^-- No need for else, just use "index++" without an else around it. If the
                                          previous condition is true, you'll break out of the loop anyway.
					index++;
				}
			}
		}
		else {
			obj = NULL;
		}
	}

	return weight;
}

EDIT: Added a couple of extra notes, and hopefully formatting is less crazy now

[Julian]

A couple of notes about the code: (I've marked inline with an ascii arrow.)

while (obj) {
		/* Curses may affect weight (index zero is original object) */
		if ((obj->weight) && (index)) weight += obj->weight;  
                ^-- You don't need to have "obj->weight" in the condition here, adding 0 doesn't change anything, so you don't need to check
                EDIT: You can also just declare "index = 1" rather than "index = 0" at the start and drop the conditional altogether
                        If index starts at 1 you can just add the weight regardless.


		/* next curse if any */
		if (curse) {
			index++;
			obj = NULL;
			while (index < z_info->curse_max) {
				if (curse[index].power) {
					obj = curses[index].obj;
					break;
				}
				else { 
                                    ^-- No need for else, just use "index++" without an else around it. If the
                                          previous condition is true, you'll break out of the loop anyway.
					index++;
				}
			}
		}
		else {
			obj = NULL;
		}
	}

EDIT: Added a couple of extra notes, and hopefully formatting is less crazy now

This entire loop structure is kind of weird. The whole process is just “look up object’s weight, and if it has curses, loop through them and add their weights”, isn’t it? That’s mostly an if and either a for or a while loop.

[Pete Mack]

That is what i thought. It's way too complex for what you'd expect to be a simple iterator.

[Nick]

using "grid->y" and "grid->x" gives me errors. But I can't pass a grid to effect_simple, I need x and y coordinates. How do I access x and y without causing errors here?

The "->" is if you have a pointer to the struct (so if grid was a struct loc *), if it's just the struct you need a ".". So grid.y and grid.x should work.

Also, the way the code does brands and slays now, I don't know how to get the brand type from the grenade to know what subtype to use with effect_simple here.

The EXPLODE flag just multiplies raw damage by 3. I think here you get to decide what subtype you want to use - the obvious possibilities are MISSILE for just raw damage, or if you want it to feel more like a physical explosion FIRE or SHARDS (or both one after the other).

[will_asher]

The EXPLODE flag just multiplies raw damage by 3. I think here you get to decide what subtype you want to use - the obvious possibilities are MISSILE for just raw damage, or if you want it to feel more like a physical explosion FIRE or SHARDS (or both one after the other).

Ordinarily, it'll be fire and shards, but grenades can get brand egos. I just don't know how to make the brand egos work with them.
(EDIT: I don't want the STRIKE effect damage to depend on damage to the particular monster it hits because other nearby monsters may have different resistances.)

[backwardsEric]

Ordinarily, it'll be fire and shards, but grenades can get brand egos. I just don't know how to make the brand egos work with them.
(EDIT: I don't want the STRIKE effect damage to depend on damage to the particular monster it hits because other nearby monsters may have different resistances.)

Can they have more than one brand? If only one is possible, then you could look through the brand array to see what ,if anything, is set, look up the related modifier and element for the brand, and use those when you invoke the effect to handle the grenade's explosion. If multiple brands are possible, I suspect you'd either have to write a custom effect handler (loop over the monsters in the range of the effect; for each, determine which brand does the most damage) or split the total base damage among the brands that are present and call one of the standard effect handlers for each of those brands.

[will_asher]

Can they have more than one brand? If only one is possible, then you could look through the brand array to see what ,if anything, is set, look up the related modifier and element for the brand, and use those when you invoke the effect to handle the grenade's explosion.

They can only get one brand.
So this is what I have so far:

in make_ranged_throw()

	/* EXPLODEing (with grenades) works separately (half FIRE and half SHARD) (but they have a chance to dud and not explode) */
	if ((of_has(obj->flags, OF_EXPLODE)) && (randint0(100) < 96 + player->p_luck)) {
		int elem = ELEM_FIRE;
		result.dmg = ranged_damage(p, mon, obj, NULL, 0, 0); /* (doubled) */
		if (obj->brands) {
			elem = grenade_brand(obj);
			/* It's already being doubled, so this makes it x3 */
			result.dmg = (result.dmg * 3 + 1) / 2;
		}
		/* Use "STRIKE" effect with "other" param to bypass asking for target and requirement of being in view of the PC. */
		effect_simple(EF_STRIKE, source_player(), format("%d", result.dmg), ELEM_SHARD, 2, 9, grid.y, grid.x, NULL);
		effect_simple(EF_STRIKE, source_player(), format("%d", result.dmg), elem, 2, 9, grid.y, grid.x, NULL); (EDIT: fixed)
		return result;
	}

with this

<EDIT: see below. snipped incomplete version>

How do I identify the brand so I know what this needs to return?
EDIT: okay, here's what I did:

/* Get a brand from a grenade */
int grenade_brand(struct object* obj)
{
	int i;
	/* Brands (copied from improve_attack_modifier()) */
	for (i = 1; i < z_info->brand_max; i++) {
		struct brand* b = &brands[i];

		if (!obj->brands[i]) continue;
		if (b->resist_flag == RF_IM_ACID) return ELEM_ACID;
		if (b->resist_flag == RF_IM_COLD) return ELEM_COLD;
		if (b->resist_flag == RF_IM_FIRE) return ELEM_FIRE;
		if (b->resist_flag == RF_IM_ELEC) return ELEM_ELEC;
		if (b->resist_flag == RF_IM_POIS) return ELEM_POIS;
	}
	/* shouldn't get here because this is only called if there's a brand on the grenade */
	return ELEM_FIRE;
}

EDIT: The "other" parameter doesn't seem to be getting passed to the STRIKE function because it's still asking for a direction (3 times...)