Add Sprite changes to Logic Gates to show the input/output state #33277

0tito · 2024-11-12T13:53:15Z

About the PR

Gates now show the state of their inputs and outputs on the sprite

Why / Balance

Its easier to use them if you can see the states

Media

2024-11-12.10-41-36.mp4

Requirements

[ x] I have read and am following the Pull Request and Changelog Guidelines.
[ x] I have added media to this PR or it does not require an ingame showcase.

Changelog

github-actions · 2024-11-12T13:53:52Z

RSI Diff Bot; head commit 16a2e13 merging into 8776c71
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Objects/Devices/gates.rsi

State	Old	New	Status
logic			Modified
empty			Added
logic_a			Added
logic_b			Added
logic_o			Added

Edit: diff updated after 16a2e13

deltanedas · 2024-11-12T21:49:42Z

use separate layers for a/b/output and none of the evil stuff is needed

0tito · 2024-11-14T02:51:45Z

Is this better? im not exactly the best coder :godo:

Aeshus

Thanks for the PR!

There's some small code style issues, and a bug where the code accidentally sets InputA instead of InputB.

Other than that, I like the change and the rest of the visualizer/layering stuff looks good (I don't have personal experience with any of it).

Also, I'm not a maintainer, so this is just a peer review. I may have made some mistakes in my review, or have missed things, so take this review with a grain of salt.

Aeshus · 2024-11-18T17:15:31Z

Content.Server/DeviceLinking/Components/LogicGateComponent.cs

+
+

Inconsistent spacing -- there should only be one line between fields.

Suggested change

Aeshus · 2024-11-18T17:16:05Z

Content.Server/DeviceLinking/Components/LogicGateComponent.cs

@@ -18,6 +18,19 @@ public sealed partial class LogicGateComponent : Component
    [DataField]
    public LogicGate Gate = LogicGate.Or;

+


Inconsistent spacing -- there should only be one line between fields.

Suggested change

Aeshus · 2024-11-18T17:16:17Z

Content.Server/DeviceLinking/Components/LogicGateComponent.cs

+    [DataField]
+    public LogicGateState Output = LogicGateState.Low;
+
+


Inconsistent spacing -- there should only be one line between fields.

Suggested change

Aeshus · 2024-11-18T17:24:43Z

Content.Server/DeviceLinking/Systems/LogicGateSystem.cs

        {
            case LogicGate.Or:
                output = a || b;
+                _appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);
                break;
            case LogicGate.And:
                output = a && b;
+                _appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);
                break;
            case LogicGate.Xor:
                output = a != b;
+                _appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);
                break;
            case LogicGate.Nor:
                output = !(a || b);
+                _appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);
                break;
            case LogicGate.Nand:
                output = !(a && b);
+                _appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);
                break;
            case LogicGate.Xnor:
                output = a == b;
+                _appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);
                break;
        }


You can extract down the _appearance.SetData call below the switch, as it's the same for every code path.

Also, there's a missing default case.

Suggested change

{

case LogicGate.Or:

output = a || b;

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

break;

case LogicGate.And:

output = a && b;

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

break;

case LogicGate.Xor:

output = a != b;

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

break;

case LogicGate.Nor:

output = !(a || b);

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

break;

case LogicGate.Nand:

output = !(a && b);

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

break;

case LogicGate.Xnor:

output = a == b;

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

break;

}

{

case LogicGate.Or:

output = a || b;

break;

case LogicGate.And:

output = a && b;

break;

case LogicGate.Xor:

output = a != b;

break;

case LogicGate.Nor:

output = !(a || b);

break;

case LogicGate.Nand:

output = !(a && b);

break;

case LogicGate.Xnor:

output = a == b;

break;

default:

throw new ArgumentOutOfRangeException();

}

_appearance.SetData(uid, LogicGateVisuals.Output, output ? LogicGateState.High : LogicGateState.Low);

If you're adventurous, you can also just refactor the switch statement into a switch expression

Aeshus · 2024-11-18T23:34:01Z

Content.Server/DeviceLinking/Systems/LogicGateSystem.cs

@@ -3,6 +3,7 @@
 using Content.Shared.DeviceLinking;
 using Content.Shared.Examine;
 using Content.Shared.Interaction;
+using Content.Shared.Lathe;


Unused import?

Suggested change

using Content.Shared.Lathe;

Aeshus · 2024-11-18T23:34:36Z

Content.Server/DeviceLinking/Systems/LogicGateSystem.cs

        }
        else if (args.Port == comp.InputPortB)
        {
            comp.StateB = state;
+            _appearance.SetData(uid, LogicGateVisuals.InputA, UpdateState(state));


Shouldn't this be modifying InputB instead of InputA?

Suggested change

_appearance.SetData(uid, LogicGateVisuals.InputA, UpdateState(state));

_appearance.SetData(uid, LogicGateVisuals.InputB, UpdateState(state));

Aeshus · 2024-11-18T23:39:09Z

Content.Server/DeviceLinking/Systems/LogicGateSystem.cs

+    LogicGateState UpdateState(SignalState state)
+    {
+        switch (state)
+        {
+            case SignalState.High:
+                return LogicGateState.High;
+            break;
+            case SignalState.Low:
+                return LogicGateState.Low;
+            break;
+            default:
+                return LogicGateState.Momentary;
+            break;
+
+        }


You're missing an access modifier on the function.

Also, you can make it more succinct by using a switch expression.

Finally, it makes sense to rename the function to SignalToLogicState, as the previous name kind-of implies some kind of side effect ("UpdateState"), when really all it does is map all of the SignalStates to their correct LogicGateState.

Suggested change

LogicGateState UpdateState(SignalState state)

{

switch (state)

{

case SignalState.High:

return LogicGateState.High;

break;

case SignalState.Low:

return LogicGateState.Low;

break;

default:

return LogicGateState.Momentary;

break;

}

private static LogicGateState SignalToLogicState(SignalState state)

{

return state switch

{

SignalState.High => LogicGateState.High,

SignalState.Low => LogicGateState.Low,

_ => LogicGateState.Momentary,

};

Aeshus · 2024-11-18T23:51:31Z

Resources/Prototypes/Entities/Structures/gates.yml

@@ -63,6 +69,18 @@
          Nor: { state: nor }
          Nand: { state: nand }
          Xnor: { state: xnor }
+      enum.LogicGateVisuals.InputA:
+        enum.LogicGateLayers.InputA:
+          High: {state: logic_a }


Missing a space.

Suggested change

High: {state: logic_a }

High: { state: logic_a }

Aeshus · 2024-11-18T23:51:48Z

Resources/Prototypes/Entities/Structures/gates.yml

@@ -36,6 +36,12 @@
    layers:
    - state: base
    - state: logic
+    - state: empty
+      map: ["enum.LogicGateLayers.InputA"]


Missing spacing.

Suggested change

map: ["enum.LogicGateLayers.InputA"]

map: [ "enum.LogicGateLayers.InputA" ]

…o the function UpdateState

Add Sprite changes to Logic Gates to show the input/output state

464f0e5

github-actions bot added the Changes: Sprites Changes: Might require knowledge of spriting or visual design. label Nov 12, 2024

Separated layers for each input/output, now a bit less evil

afa0b03

SlamBamActionman added the S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. label Nov 14, 2024

Aeshus suggested changes Nov 19, 2024

View reviewed changes

Removed unused dependency, fixed inconsistent spacings, and changes t…

16a2e13

…o the function UpdateState

github-actions bot added the size/M Denotes a PR that changes 30-99 lines. label Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Sprite changes to Logic Gates to show the input/output state #33277

Add Sprite changes to Logic Gates to show the input/output state #33277

0tito commented Nov 12, 2024

github-actions bot commented Nov 12, 2024 •

edited

Loading

deltanedas commented Nov 12, 2024

0tito commented Nov 14, 2024

Aeshus left a comment

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

Aeshus Nov 18, 2024

		@@ -18,6 +18,19 @@ public sealed partial class LogicGateComponent : Component
		[DataField]
		public LogicGate Gate = LogicGate.Or;

		[DataField]
		public LogicGateState Output = LogicGateState.Low;

	_appearance.SetData(uid, LogicGateVisuals.InputA, UpdateState(state));
	_appearance.SetData(uid, LogicGateVisuals.InputB, UpdateState(state));

	map: ["enum.LogicGateLayers.InputA"]
	map: [ "enum.LogicGateLayers.InputA" ]

Add Sprite changes to Logic Gates to show the input/output state #33277

Are you sure you want to change the base?

Add Sprite changes to Logic Gates to show the input/output state #33277

Conversation

0tito commented Nov 12, 2024

About the PR

Why / Balance

Media

Requirements

github-actions bot commented Nov 12, 2024 • edited Loading

deltanedas commented Nov 12, 2024

0tito commented Nov 14, 2024

Aeshus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 12, 2024 •

edited

Loading