Bug - Fixed Modifier Handling for Evaluated Modifiers dependant on Turns

Darzil

Developer
This one is driving me a little insane, so advice, thoughts, help, investigation welcomed.

It appears to me at present that we create Modifiers in different ways. A Modifier has two pieces of data, a name and a value. Now, sometimes in constructing the modifier, the name is the name of the thing producing the modifier, eg Effect:Toxic Vengeance or Effect:[1818], and sometimes it is the name of the modifier being Modified, eg Muscle.

The only place I've found so far where the name really matters is that the name is used to work out if a character is under an effect. In this case, the first type of constructor works, the second fails.

My gut feeling is we should either have two different types of modifier, or an extra value, parent, which is filled in for the second type of constructor, and used for working out effect where present.

Either way, it looks messy, so would really appreciate a second pair of eyes!
 

Veracity

Developer
Staff member
Now, sometimes in constructing the modifier, the name is the name of the thing producing the modifier, eg Effect:Toxic Vengeance or Effect:[1818], and sometimes it is the name of the modifier being Modified, eg Muscle.
If you are referring to class "Modifier" in Modifiers.java, looking at the code, grep is not showing me any examples of the first type being created.

Code:
./Modifiers.java:2104:			this.list.add( new Modifier( name, value ) );
./persistence/DebugDatabase.java:1390:		return new Modifier( key, value );
The first is this in class ModifierList:

Code:
		public void addModifier( final Modifier modifier )
		{
			this.list.add( modifier );
		}

		public void addModifier( final String name, final String value )
		{
			this.list.add( new Modifier( name, value ) );
		}
The second is this in DebugModifiers:

Code:
	private static final Modifier makeModifier( final String mod )
	{
		int colon = mod.indexOf( ":" );
		String key = colon == -1 ? mod.trim() : mod.substring( 0, colon ).trim();
		String value = colon == -1 ? null : mod.substring( colon + 1 ).trim();
		return new Modifier( key, value );
	}
In both cases, I see no places where the "key" or "name" is ever anything other than the name of a modifier; it never seems to be the name of an object.

Where are you seeing a Modifier object created with the name of an object, not the name of a modifier?
 

Veracity

Developer
Staff member
Yeah.

A Modifiers object has a name which is something like "Effect:[1818]"
A Modifier object has a name which is something like "Muscle".

Looking at the various places where Modifiers.getNameFromLookup (which is new to me) is called, sometimes it is used within a Modifers object (which will work as expected) and sometimes from within a Modifier object (which is pointless, since a Modifier object never has a type:name in it - just a name).

I'll see if I can straighten this out.
 

heeheehee

Developer
Staff member
I started to debug this for the heck of it. The problem *looks* like when a Modifier instance is created, it has no knowledge of the effect (or other source) it corresponds to.

Actually, I think I know what's up. The "name" being passed in to ModifierExpression in Modifier.eval is the name of the modifier, rather than the name of the source.
 

Veracity

Developer
Staff member
OK, the only confused place seems to be in the eval() method of a modifier, which makes a ModifierList and uses the the name of the Modifier itself as the name for the ModifierList, which actually needs the name of the object. We need to pass the name of the object down to that method. Working on it.
 

Veracity

Developer
Staff member
Yup!

OK, revision 15707 hopefully fixes this. I can't test it today, since I am out of turns and no longer have Toxic Vengeance; I ran enough turns today in the Toxic Teacups to get 650 toxic globules, which sufficed to buy the items I wanted, at which point I SGEEA'd the effect and used the rest of my turns elsewhere. If one of you has turns of Toxic Vengeance active, I'm curious to see what the following reports:

ash string_modifier($effect[toxic vengeance], "Evaluated Modifiers")
 

Darzil

Developer
Thank you! Was driving me bad trying to work out what should happen, and why
ash string_modifier($effect[toxic vengeance], "Evaluated Modifiers") was giving such odd values!

I have turns and Toxic Vengeance, so will test.
 

Darzil

Developer
Not there yet, unfortunately.

Returned: Muscle: 0, Mysticality: 0, Moxie: 0, Stench Damage: 0, Spell Damage Percent: 0
 

Darzil

Developer
I'll look at it from here if you like, as I can test. Looks like it is being passed the lookup rather than the name. That might be right, but if so we just have to convert it before seeing if it is in the effect database.
 

Veracity

Developer
Staff member
Heh. That's what it shows for me, which is correct - for me. :)

In ModifierExpression.java, change initialize() to this:

Code:
	protected void initialize()
	{
		// The first check also matches "[zone(The Slime Tube)]"
		// Hence the second check.
		int effectId = EffectDatabase.getEffectId( Modifiers.getNameFromLookup( this.name ) );
		if ( text.contains( "T" ) && effectId != -1 )
		{
			this.effect = EffectPool.get( effectId, 0 );
		}
	}
i.e. take the passed in object name and extract the "name" from it before lookup up the effect name. Hopefully that will replace the call to Modifiers.getNameFromLookup that I removed from Modifier.eval (which did not have an object name, just a modifier name).

If it works, check it in and declare this fixed.
 

Veracity

Developer
Staff member
I'll look at it from here if you like, as I can test. Looks like it is being passed the lookup rather than the name. That might be right, but if so we just have to convert it before seeing if it is in the effect database.
I replied before I saw that you had diagnosed the same thing. I think passing in the Lookup is correct, so that we can distinguish between Items and Effects, if we wish. Of course, in that case, we could not bother getting this.effect unless it is Effect:something, right?

Edit: like this:

Code:
	@Override
	protected void initialize()
	{
		String type = Modifiers.getTypeFromLookup( this.name );
		// The second check also matches "[zone(The Slime Tube)]"
		// in which case, we don't need the effect. Big deal.
		if ( type.equals( "Effect" ) && text.contains( "T" ) )
		{
			int effectId = EffectDatabase.getEffectId( Modifiers.getNameFromLookup( this.name ) );
			if ( effectId != -1 )
			{
				this.effect = EffectPool.get( effectId, 0 );
			}
		}
	}
 
Last edited:

Darzil

Developer
Yup. It does fix that bug:
Code:
> ash string_modifier($effect[toxic vengeance], "Evaluated Modifiers")

Returned: Muscle: -5, Mysticality: -5, Moxie: -5, Stench Damage: +15, Spell Damage Percent: +15
but introduces another, as Toxic Vengeance is no longer being applied in Mafia more generally now. Will investigate.
 
Top