Make status effect networking not use TryAddStatusEffect

[ElectroJr]

This PR is meant to try stop client side errors like this one:

A side effect is that this no longer raises status effect add & remove events during state handling, under the assumption that the effects that those events would normally have would result in modifying networked components (i.e., the state handling should already apply those effects). If that's not the case, like for non-networked client side effects (i.e. flashbang), they can just associate a component and use comp add/remove events.

[slarticodefast]

This does not work with flashes due to prediction issues. Your solution in FlashSystem.cs is the same I tried first when changing it to use the status effect system, but prediction causes the callback in state.Viewport.Viewport.Screenshot(...) in the ReceiveFlash() function to be used multiple times, sometimes even after the status effect alread has disappeared, causing it to use a wrong screenshot. Note that this cannot be observed when using breakpoints, but only when logging to the console with sawmill.

I have an alternative idea to completely get rid of imagesharp in the overlay and use RequestScreenTexture instead. Because this is quite costly performance-wise, an engine change would be needed to allow changing that bool to true only when needed.

[ElectroJr]

But prediction causes the callback in state.Viewport.Viewport.Screenshot(...) in the ReceiveFlash() function to be used multiple times

The same thing happens with the the old status effect event - it should get raised repeatedly whenever the client predicts that an effect started or ended. So AFAIK this shouldn't change the behaviour at all.

A way to fix it might be to have the flash system capture or clear the screenshot during a frameupdate, so that it only happens during the first frame after the component was added, but I'd say reworking that is outside of the scope of this PR

Edit:
Actually it seems like the event handler was only firing once, but was only doing so accidentally due to incidental behaviour of the order in which the state handler applies game states. The even handler was subscribing with the FlashedComponent, which only gets added after the status effect gets added and the after the StatusEffectAddedEvent has already been raised. It just so happens that client state handling adds all new components before applying all comp states for that tick. So the exact kind of subscription on a server-side system wouldn't ever work, and and generally couldn't be used on the client for any predicted effects that the status effect should have. I,.e. subscriptions to StatusEffectAddedEvent shouldn't use the component that gets added by the status effect unless status effects are reworked to change the order in which the comp gets added & the event gets raised.

[slarticodefast]

Having it done in the frame update is not that simple because the callback takes a few frames. It would need some extra logic to ensure only one screenshot is taken. Sometimes lag even causes the screenshot to be taken even after the component and overlay have been removed.

I don't know how urgent this is, but I can do a separate PR in the next few days to get rid of the the callback completely, which should solve this problem.

[ElectroJr]

I don't know how urgent this is

I don't think its all that urgent? It is the first time I've seen the error reported, and I don't even know what caused it.

[slarticodefast]

@ElectroJr Done!
#28930

[slarticodefast]

This will need to be adjusted now that #28930 is merged. No idea why GitHub did not trigger a merge conflict.