FakeFishGames / Barotrauma

A 2D online multiplayer game taking place in a submarine travelling through the icy depths of Jupiter's moon Europa.
http://www.barotraumagame.com/
1.74k stars 403 forks source link

Wiring/signal component timing issues, circuit behavior affected by the creation order of the components #12993

Open Regalis11 opened 11 months ago

Regalis11 commented 11 months ago

Disclaimers

What happened?

Due to the way to the signal system works, the order in which components receive signals can sometimes affect the behavior of a circuit. The order in which components are processed depends on their creation order, which means the creation order of the components can affect the behavior.

This can happen in e.g. these situations:

  1. If the circuit is very complex and the signal propagation doesn't get fully calculated during one frame.
  2. If the order in which the signals arrive to the item makes a difference, and the signal needs to travel a longer distance to one of the connections.
  3. If an input is receiving signals from two different sources during the same frame.

One common situation are memory components. When you set the value of a memory component, it does not immediately pass this value forwards, but instead stores the value and sends it the next frame when it's processed by the signal system. If the circuit is built in a way where e.g. the component is receiving signals from multiple sources, or getting locked on the same tick as the value is being set, the behavior may depend on which component sets the value last, or whether some other component locks the memory component before the value is set.

I would see the behavior in the 3rd situation as something that isn't necessarily a bug - if the item is receiving signals from multiple sources at the same time, the behavior arguably should be unpredictable. If one signal is telling a memory component to set the value to "A", and another is telling to set it to "B", what should it be set to?

The first 2 situations are clearly issues though. However, this has turned out to be quite a complex problem to solve, and so far we haven't come up with a feasible way to solve it (aside from redesigning the whole signal logic from the ground up and most likely breaking tons of custom circuits in the process). It also hasn't been that high on our priority list, since it seems to affect a very small portion of players building very specific kinds of complex circuits.

There was one https://github.com/Regalis11/Barotrauma/discussions/5001 that we considered for a while, but it turned out it had various other problems that would've been possibly even worse than the issues with the current system.

Reproduction steps

Some existing reports with descriptions of circuits affected by the issue:

Bug prevalence

Happens every now and then

Single player or multiplayer?

Happens in both single player and multiplayer

-

No response

Version

v1.1.19.3 (Treacherous Tides Hotfix 2)

-

No response

Which operating system did you encounter this bug on?

Windows

Relevant error messages and crash reports

No response

Boolean-Buckeye commented 11 months ago

I am glad that this issue is still on the radar of the development, because it has been pretty much my main outstanding issue with this game for a long time. I made a similar issue report a while back, I am not sure what happened to it. But, this issue is actually probably the main reason I quit playing this game some months ago (although to be fair, I tend to binge a game for a while then randomly stop playing). I personally believe that avoiding breaking custom circuits built in a faulty system should not be a higher priority than making the system coherent, but, if this is really an issue, perhaps there can be some kind of configuration option to choose between old/new wiring logic, maybe as part of submarine editor? (since the reason to choose between old/new depends more on submarine design than server/user preference)

Regalis11 commented 10 months ago

I personally believe that avoiding breaking custom circuits built in a faulty system should not be a higher priority than making the system coherent, but, if this is really an issue

I disagree with this. I would argue the timing issues tend to be a pretty rare problem that only occurs in specific kinds of (often very complex) circuits, and we should not risk breaking circuits in potentially thousands and thousands of existing custom subs to address a rare issue like this.

In my opinion the case you reported seems like one of the situations where arguably the issue is in the circuit itself, not necessarily a fault in the system: in a circuit where the memory component is receiving signals to both "signal_in" and "lock_state" on the same frame, it's essentially told to store a value and not store a value at the same time. What would the expected behavior be in this situation? We could most likely add some kind of a rule for this, e.g. that either connection always takes precedence, but which connection should that be?

Boolean-Buckeye commented 10 months ago

in a circuit where the memory component is receiving signals to both "signal_in" and "lock_state" on the same frame, it's essentially told to store a value and not store a value at the same time.

From the perspective of an electrical engineer, the memory component looks a lot like a gated D latch https://en.wikipedia.org/wiki/Flip-flop_(electronics)#Gated_D_latch. I think at the time I made that circuit in that issue, there was only one connection to the memory lock signal, but perhaps I was considering it more like a D flip flop https://en.wikipedia.org/wiki/Flip-flop_(electronics)#Classical_positive-edge-triggered_D_flip-flop where the frame change was like a clock edge, and that's why I expected the pulse signal to work (expected pulse to last a whole frame, I guess). So I think it is possible I just made a wrong assumption about the way Barotrauma wiring works, but I don't remember very well.

Regalis11 commented 10 months ago

That makes sense - personally I think of the memory component as something more complex than a latch that deals with binary values, maybe something akin to a flash drive that can store arbitrary data and can be set to a read-only mode. And with a component like that, flipping it to read-only mode while trying to store something in the memory could have unpredictable results, or at the least be very timing-sensitive.

But this behavior with memory components is something that comes up somewhat frequently, so I think we could consider adding some kind of a rule to make the behavior consistent. I think the question is just which connection should take precedence: should the new value be stored even if a signal to make it read-only was received earlier that frame, or should a signal to the lock_state input invalidate all signals received that frame?

DarthPointer commented 10 months ago

The entire system needs replacement if it happens that other components may both delay and not signals for frames depending on their creation order. If I create component B first, then A and link the output of A into B, the B will update first with A updating and pushing value to only be processed by B next frame, is this how the implementation works?

By replacement I mean a separate set of signals that have their behaviour well-determined with existing tools and not requiring absurd control over circuit creation process. Systems that rely on current quirks may exist and they should not be assumed to be under active maintenance and support, so they should be let stay as is.

Regalis11 commented 10 months ago

I agree that a player should not need to know or care about the creation order of the components, but as I mentioned, if you're sending two conflicting signals two a component at the same time, I see that as a situation that arguably should lead to unpredictable behavior, and that you shouldn't be trying to work around the issue by relying on the update order of the items. What would you expect the behavior to be in the case of the memory component - which signal should take precedence?

DarthPointer commented 10 months ago

What would you expect the behavior to be in the case of the memory component - which signal should take precedence?

Probably declare it as "undefined", and be free to implement input merging whatever provides better optimisation and maintainability.

mygamingaccount commented 9 months ago

Due to the way to the signal system works, the order in which components receive signals can sometimes affect the behavior of a circuit. The order in which components are processed depends on their creation order, which means the creation order of the components can affect the behavior.

This can happen in e.g. these situations:

1. If the circuit is very complex and the signal propagation doesn't get fully calculated during one frame.

I have not seen this happen, instead I've seen Bad Apple animated on light components using regular expressions and signals with lengths in many kilobytes 😄

  1. If the order in which the signals arrive to the item makes a difference, and the signal needs to travel a longer distance to one of the connections.

  2. If an input is receiving signals from two different sources during the same frame.

One common situation are memory components. When you set the value of a memory component, it does not immediately pass this value forwards, but instead stores the value and sends it the next frame when it's processed by the signal system. If the circuit is built in a way where e.g. the component is receiving signals from multiple sources, or getting locked on the same tick as the value is being set, the behavior may depend on which component sets the value last, or whether some other component locks the memory component before the value is set.

The memory components pass the values immediately (in the same frame, or tick) unless it receives a signal from a component which is updated late. This behavior is identical to every other component, Memory is not an exception.

Example: Adder before Memory, in one tick:

  1. Adder.Update() runs, Calculate()'s the inputs, eventually calls MemoryComponent.ReceiveSignal() via Adder.SendSignal(), this changes MemoryComponent.Value
  2. MemoryComponent.Update() runs, this calls MemoryComponent.SendSignal() and thus every connected item's ReceiveSignal() method with the new value

Memory before Adder:

  1. Memory.Update() runs and sends the old value to every connected item, which was altered in a previous frame
  2. Adder.Update() runs, sends a signal that changes MemoryComponent.Value

As you can see the first example results in no delay, while the second example results in a single sample delay, which is a very useful component in digital signal processing functions.

I would see the behavior in the 3rd situation as something that isn't necessarily a bug - if the item is receiving signals from multiple sources at the same time, the behavior arguably should be unpredictable. If one signal is telling a memory component to set the value to "A", and another is telling to set it to "B", what should it be set to?

I agree that it is not really a bug. Since Signals are separate from Wires due to the possibility of there being no signal on a wire, multiple connections to the same input can be valid. Multiple connections to the same input are also valid when a component can handle it: for example logic components can handle many simultaneous incoming signals in the same frame, and work like such: if an input gets at least one non-zero signal, then that input becomes active. Afterwards the output will become active if a) one input is active (XOR) b) two inputs are active (AND) c) either input is active (OR) So what is expected when there are two conflicting continuous signals going to the same pin which can only support one signal per frame like Memory? While I agree that the outcome is counter intuitive, I think that is user error and not a bug.

The way I see it, for the wiring system to be as useful and as it is currently, as well as both intuitive and predictable, a discrete-time subset of Matlab's Simulink, or Scilab's XCOS would be required. These compile the entire block diagram of components and signal items into a function, while performing checks to identify or break up algebraic loops, then execute them to get the outputs. Is that a realistic expectation from a game about submarines in space? 🤷