Changing Clash Speed On Proffie Saber

ProffieOS
21

So, I’ll just move the if() from Clash2() into Clash3() in prop_base and then leave the override in my prop to just IgnoreClash(100)? Is that the better approach?

22

Or should I just make Clash2 virtual and override it?

23

Clash2 is already virtual, so you could override that directly.
I think it should look like this though:


  virtual void Clash2(bool stab, float strength) {
    SaberBase::SetClashStrength(strength);
    if (Event(BUTTON_NONE, stab ? EVENT_STAB : EVENT_CLASH)) {
      IgnoreClash(400);
    } else {
      IgnoreClash(100);
      Clash3(stab);
    }
  }

  virtual void Clash3(bool stab) {
      // Saber must be on and not in lockup mode for stab/clash.
      if (SaberBase::IsOn() && !SaberBase::Lockup()) {
        if (stab) {
          SaberBase::DoStab();
        } else {
          SaberBase::DoClash();
        }
      }
  }

Then you override Clash3() to do all your clash handling and remove anything that tries to handle clashes from Event() / Event2().

24

Wouldn’t this still add the 400ms where I use HandleClash() if I only override Clash3() or am I missing something?

25

Just a thought, what if the above if statement and IgnoreClash(400) was made an override in the props that need clash-for-action instead?

26

It would only use the 400ms if Event(BUTTON_NONE, EVENT_CLASH) returns true.
If you remove the clash handling from Event(), then it will not return true.

27

Ok, so I’ll have to rewrite all of the events using Clash and Stab, is that the idea?

28

Yes, basically your in your prop, you’re changing it from: (psuedocode)

bool Event2(...) {
   case CLASH:
      clash code;
      return true;

  case STAB;
      stab code;
      return true;
}

to

void Clash3(bool stab) override (
   if (stab) {
      stab code;
   } else {
      clash code;
   }
}
29

Ok, might take some time, there’s a lot on those events due to Edit Mode, Battle Mode and Choreography Mode.
Are you sure it isn’t simpler to just move the IgnoreClash(400) check into props that need it for clash-for-action?

30

One possible alternative would be to just change Clash2() to always do IgnoreClash(100). We would then need to add IgnoreClash(400) inside Event2 when we match an event that needs more protection from duplicate clashes.

Basically, instead of having Clash2() figure out the right ignore time, we just delegate it.

It makes things super-easy for your prop, but requires a little work in other places.

31

What if I just do this in my prop and leave everything else alone, is there any downside to using the Event to handle DoEffect?

void Clash2(bool stab, float strength) override {
    SaberBase::SetClashStrength(strength);
    IgnoreClash(100);
}

I think everything else for the effects, etc. is already handled in my prop to determine clash, stab, lockup, etc.
The EVENT_CLASH, EVENT_STAB come prior to Clash2, correct?

32

You need to add the call to Event() in there, but other than that, it should work fine.
It’s a little bit of a buck-passing thing since it doesn’t really solve the problem, but it does work around in a perfectly reasonable way.

33

Ok, just looking at everything in those Events it seemed more effective to just address the IgnoreClash(400) portion than try to rewrite all of the existing code since that’s really the only piece that’s not working as expected. Everything else is running perfectly, it is just that added delay that needed addressing, since it is odd to have the delay for actual clashes.
I’ll test it out, there’s a few PRs in front of this bit if it works as expected I’ll get it submitted.

34

Sure. that’s not my goal though.
The goal is to make the code work well and be easy to understand.
Software engineering is absolutely shock full of people doing things the easy way and paying for it later. In this case it seems that we’ve found an easy way that doesn’t hurt anything in terms of functionality or how the code works, and in that case, doing the easy thing is the right choice.

35

Understood.