Help me make my new variant! (please!)

[Grenoble]

Oh nice! Subscribed to this thread, please post here when you release. Congrats!

Some of the best documentation I found on building older software came from forums like this. Anything you can post on your process would be interesting to me.

[will_asher]

Still messing around with this? I just went down this rabbit hole with the ToME variant. I wanted to build it on Win 10. I could sort of read c++ when I started and now I know how to debug a linking problem with a library. It was an amazing journey and I hope you'll keep at it. Here are some things I'd wish I knew, etc:

Thanks for the tips. I am pretty close to releasing an alpha version for other people to play (and help me find & fix bugs).
I've been using Visual Studio 2019, and I don't touch the make files. I've never used cmake.

[Grenoble]

Still up?

So, I'm looking into making a new variant since I just came back to Angband, and I always think the most fun thing to do with a game is tweak it, tinker with it, tamper with it, customize it, alter it, twist it, slime it, and otherwise modify it. But I'm going to need some help because I am much more of a hack than an actual coder/programmer, so this thread is where I'll ask questions.

Still messing around with this? I just went down this rabbit hole with the ToME variant. I wanted to build it on Win 10. I could sort of read c++ when I started and now I know how to debug a linking problem with a library. It was an amazing journey and I hope you'll keep at it. Here are some things I'd wish I knew, etc:

• I'd really recommend starting with a version that uses cmake. The integration with vs 2019 exposes a lot of features it would have taken forever to find on my own. It was easier for me to "get" cmake and then go back and understand older makefiles.
  
• C++ encompasses C (I didn't/still don't really get this), so VS will see your project as C++. I forked someone's code and then created a new project from existing files.
  
• The compiler you'll end up using is determined by the "triplet" you're targeting and the platform you're building on. Triplet is a standard term used in cross compiling as a way to completely capture the target environment (cpu, os, compiler, runtime, etc) in a single convenient name. If you were building on windows for linux, for example, you might use gcc through mingw.
  
• The link above is from the home of a package manager that can handle common libraries and tools for windows. It turned into a good resource for me.
  
• Most of the documentation you'll run into is hot garbage. Find someone you can talk to when you are well and truly stuck.

[Nick]

As a side note, one usually shouldn’t pass structs directly, as it makes the compiler copy it onto the stack. Pass by reference with a const declaration instead. (Not that it’s a big deal for a grid, which is tiny.)

Yeah, in most of these case the struct loc has replaced two int arguments, so it's pretty much neutral.

[Julian]

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.

As a side note, one usually shouldn’t pass structs directly, as it makes the compiler copy it onto the stack. Pass by reference with a const declaration instead. (Not that it’s a big deal for a grid, which is tiny.)

[backwardsEric]

Reading out the element from the resist looks correct. You didn't pass the result of grenade_brand() as the element for the effect (I assume you would want it to replace ELEM_FIRE). If you want distinguish the weak and strong brands, you'd want to get the multiplier as well.

As for the targeting, the "other" parameter doesn't affect that for EF_STRIKE. Someone else will have to chime in how to best set the target for this case.

[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...)

[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]

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.)

[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).

[Pete Mack]

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

[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.

[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

[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.

[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.