Solaris-Skunk-Werks / solarisskunkwerks

66 stars 26 forks source link

Shield -1MP should be cumulative and also apply to jumping MP. #308

Open TonicArtos opened 1 year ago

TonicArtos commented 1 year ago

Medium and Large Shields have -1MP. This applies to all forms of movement as per questions answered. SSW does not apply -1MP to jumping movement.

Also.

-1MP should be applied for each shield. SSW only applies a single -1 modifier.

To Reproduce

  1. Add jumping MP to base chassis.
  2. Add two Medium Shields
  3. Observe only -1MP to walking MP. (should be -2)
  4. Observe no reduction of jumping MP. (currently 0 but fixed in pull for #305)

and

  1. Add jumping MP to base chassis.
  2. Add two Large Shields
  3. Observe only -1MP to walking MP. (should be -2)

Expected behavior

  1. Add jumping MP to base chassis.
  2. Add two Medium Shields
  3. Observe -2MP to walking MP.
  4. Observe -2 MP to jumping MP.

and

  1. Add jumping MP to base chassis.
  2. Add two large Shields
  3. Observe -2MP to walking MP.
  4. Observe 0 jumping MP.
TonicArtos commented 1 year ago

I think it is related to the arraylist for storing mech modifiers.

Mech.java:4834

    public void AddMechModifier( MechModifier m ) {
        if( m == null ) { return; }
        if( ! MechMods.contains( m ) &! CurLoadout.GetMechMods().contains( m ) ) {
            MechMods.add( m );
        }
    }

I think the json generated MechModifiers cannot be distinguished by the ArrayList default comparator. As such, subsequent modifiers added to the list are ignored. There is additional behaviour that supports this, adding and then removing a second shield restores movement values, but leaves the first shield in the equipment list.