From d40bcc9168610c31aa117af61e3171aa0fe3305c Mon Sep 17 00:00:00 2001 From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com> Date: Mon, 31 Jan 2022 05:34:48 +1300 Subject: [PATCH] Fix some mispredict reconciliation issues. (#6319) Co-authored-by: metalgearsloth --- Content.Client/Alerts/ClientAlertsSystem.cs | 2 +- .../Visualizers/PipeConnectorVisualizer.cs | 4 +- .../Botany/PlantHolderVisualizer.cs | 12 +++-- Content.Client/Damage/DamageVisualizer.cs | 4 +- Content.Client/Rotation/RotationVisualizer.cs | 22 ++++---- .../Botany/Components/PlantHolderComponent.cs | 19 +++---- .../NodeContainer/Nodes/PipeNode.cs | 2 +- Content.Shared/Atmos/PipeDirection.cs | 13 ----- Content.Shared/Botany/PlantHolderVisuals.cs | 3 +- Content.Shared/Chemistry/SolutionVisuals.cs | 7 ++- Content.Shared/Damage/DamageVisualizerKeys.cs | 17 +++++++ .../Damage/Systems/DamageableSystem.cs | 7 ++- .../Hands/Components/SharedHandsComponent.cs | 9 +++- .../StatusEffect/StatusEffectsSystem.cs | 50 ++++++++++++------- .../Storage/Components/SharedMapLayerData.cs | 10 ++-- 15 files changed, 110 insertions(+), 71 deletions(-) diff --git a/Content.Client/Alerts/ClientAlertsSystem.cs b/Content.Client/Alerts/ClientAlertsSystem.cs index 0c189963e3..c1792923c4 100644 --- a/Content.Client/Alerts/ClientAlertsSystem.cs +++ b/Content.Client/Alerts/ClientAlertsSystem.cs @@ -76,7 +76,7 @@ internal class ClientAlertsSystem : AlertsSystem if (componentAlerts == null) return; //TODO: Do we really want to send alerts for non-attached entity? - component.Alerts = componentAlerts; + component.Alerts = new(componentAlerts); if (!CurControlled(component.Owner, _playerManager)) return; SyncAlerts?.Invoke(this, componentAlerts); diff --git a/Content.Client/Atmos/Visualizers/PipeConnectorVisualizer.cs b/Content.Client/Atmos/Visualizers/PipeConnectorVisualizer.cs index bab0d8134c..2aa0a523ab 100644 --- a/Content.Client/Atmos/Visualizers/PipeConnectorVisualizer.cs +++ b/Content.Client/Atmos/Visualizers/PipeConnectorVisualizer.cs @@ -72,7 +72,7 @@ namespace Content.Client.Atmos.Visualizers if (!component.TryGetData(PipeColorVisuals.Color, out Color color)) color = Color.White; - if (!component.TryGetData(PipeVisuals.VisualState, out PipeVisualState state)) + if (!component.TryGetData(PipeVisuals.VisualState, out PipeDirection connectedDirections)) return; if(!component.TryGetData(SubFloorVisuals.SubFloor, out bool subfloor)) @@ -84,7 +84,7 @@ namespace Content.Client.Atmos.Visualizers { var layer = sprite.LayerMapGet(layerKey); var dir = (PipeDirection) layerKey; - var visible = subfloor && state.ConnectedDirections.HasDirection(dir); + var visible = subfloor && connectedDirections.HasDirection(dir); sprite.LayerSetVisible(layer, visible); if (!visible) continue; diff --git a/Content.Client/Botany/PlantHolderVisualizer.cs b/Content.Client/Botany/PlantHolderVisualizer.cs index e9be155804..aee51cab44 100644 --- a/Content.Client/Botany/PlantHolderVisualizer.cs +++ b/Content.Client/Botany/PlantHolderVisualizer.cs @@ -58,12 +58,18 @@ namespace Content.Client.Botany var sprite = IoCManager.Resolve().GetComponent(component.Owner); - if (component.TryGetData(PlantHolderVisuals.Plant, out var specifier)) + if (component.TryGetData(PlantHolderVisuals.PlantRsi, out var rsi) + && component.TryGetData(PlantHolderVisuals.PlantState, out var state)) { - var valid = !specifier.Equals(SpriteSpecifier.Invalid); + var valid = !string.IsNullOrWhiteSpace(state); + sprite.LayerSetVisible(PlantHolderLayers.Plant, valid); + if(valid) - sprite.LayerSetSprite(PlantHolderLayers.Plant, specifier); + { + sprite.LayerSetRSI(PlantHolderLayers.Plant, rsi); + sprite.LayerSetState(PlantHolderLayers.Plant, state); + } } if (component.TryGetData(PlantHolderVisuals.HealthLight, out var health)) diff --git a/Content.Client/Damage/DamageVisualizer.cs b/Content.Client/Damage/DamageVisualizer.cs index 8ac706777b..489d63875f 100644 --- a/Content.Client/Damage/DamageVisualizer.cs +++ b/Content.Client/Damage/DamageVisualizer.cs @@ -548,9 +548,9 @@ namespace Content.Client.Damage { UpdateDamageVisuals(damageComponent, spriteComponent, damageData); } - else if (component.TryGetData>(DamageVisualizerKeys.DamageUpdateGroups, out List? delta)) + else if (component.TryGetData(DamageVisualizerKeys.DamageUpdateGroups, out DamageVisualizerGroupData data)) { - UpdateDamageVisuals(delta, damageComponent, spriteComponent, damageData); + UpdateDamageVisuals(data.GroupList, damageComponent, spriteComponent, damageData); } } diff --git a/Content.Client/Rotation/RotationVisualizer.cs b/Content.Client/Rotation/RotationVisualizer.cs index 2021c5b76a..99da3d90c2 100644 --- a/Content.Client/Rotation/RotationVisualizer.cs +++ b/Content.Client/Rotation/RotationVisualizer.cs @@ -1,4 +1,4 @@ -using System; +using System; using Content.Shared.Rotation; using JetBrains.Annotations; using Robust.Client.Animations; @@ -17,17 +17,17 @@ namespace Content.Client.Rotation { base.OnChangeData(component); - if (component.TryGetData(RotationVisuals.RotationState, out var state)) + // if TryGet fails, state defaults to RotationState.Vertical. + component.TryGetData(RotationVisuals.RotationState, out var state); + + switch (state) { - switch (state) - { - case RotationState.Vertical: - SetRotation(component, 0); - break; - case RotationState.Horizontal: - SetRotation(component, Angle.FromDegrees(90)); - break; - } + case RotationState.Vertical: + SetRotation(component, 0); + break; + case RotationState.Horizontal: + SetRotation(component, Angle.FromDegrees(90)); + break; } } diff --git a/Content.Server/Botany/Components/PlantHolderComponent.cs b/Content.Server/Botany/Components/PlantHolderComponent.cs index cf79f2839b..a423ebf43e 100644 --- a/Content.Server/Botany/Components/PlantHolderComponent.cs +++ b/Content.Server/Botany/Components/PlantHolderComponent.cs @@ -592,30 +592,31 @@ namespace Content.Server.Botany.Components if (Dead) { - appearanceComponent.SetData(PlantHolderVisuals.Plant, - new SpriteSpecifier.Rsi(Seed.PlantRsi, "dead")); + appearanceComponent.SetData(PlantHolderVisuals.PlantRsi, Seed.PlantRsi.ToString()); + appearanceComponent.SetData(PlantHolderVisuals.PlantState, "dead"); } else if (Harvest) { - appearanceComponent.SetData(PlantHolderVisuals.Plant, - new SpriteSpecifier.Rsi(Seed.PlantRsi, "harvest")); + appearanceComponent.SetData(PlantHolderVisuals.PlantRsi, Seed.PlantRsi.ToString()); + appearanceComponent.SetData(PlantHolderVisuals.PlantState, "harvest"); } else if (Age < Seed.Maturation) { var growthStage = Math.Max(1, (int) ((Age * Seed.GrowthStages) / Seed.Maturation)); - appearanceComponent.SetData(PlantHolderVisuals.Plant, - new SpriteSpecifier.Rsi(Seed.PlantRsi, $"stage-{growthStage}")); + + appearanceComponent.SetData(PlantHolderVisuals.PlantRsi, Seed.PlantRsi.ToString()); + appearanceComponent.SetData(PlantHolderVisuals.PlantState, $"stage-{growthStage}"); _lastProduce = Age; } else { - appearanceComponent.SetData(PlantHolderVisuals.Plant, - new SpriteSpecifier.Rsi(Seed.PlantRsi, $"stage-{Seed.GrowthStages}")); + appearanceComponent.SetData(PlantHolderVisuals.PlantRsi, Seed.PlantRsi.ToString()); + appearanceComponent.SetData(PlantHolderVisuals.PlantState, $"stage-{Seed.GrowthStages}"); } } else { - appearanceComponent.SetData(PlantHolderVisuals.Plant, SpriteSpecifier.Invalid); + appearanceComponent.SetData(PlantHolderVisuals.PlantState, ""); appearanceComponent.SetData(PlantHolderVisuals.HealthLight, false); } diff --git a/Content.Server/NodeContainer/Nodes/PipeNode.cs b/Content.Server/NodeContainer/Nodes/PipeNode.cs index 8fba7ce342..e362170f0e 100644 --- a/Content.Server/NodeContainer/Nodes/PipeNode.cs +++ b/Content.Server/NodeContainer/Nodes/PipeNode.cs @@ -327,7 +327,7 @@ namespace Content.Server.NodeContainer.Nodes } } - appearance.SetData(PipeVisuals.VisualState, new PipeVisualState(netConnectedDirections)); + appearance.SetData(PipeVisuals.VisualState, netConnectedDirections); } } } diff --git a/Content.Shared/Atmos/PipeDirection.cs b/Content.Shared/Atmos/PipeDirection.cs index 598c9d9878..458908c167 100644 --- a/Content.Shared/Atmos/PipeDirection.cs +++ b/Content.Shared/Atmos/PipeDirection.cs @@ -11,19 +11,6 @@ namespace Content.Shared.Atmos VisualState } - [Serializable, NetSerializable] - public class PipeVisualState - { - // TODO ATMOS: Make this not a class and just be the field below... - [ViewVariables] - public readonly PipeDirection ConnectedDirections; - - public PipeVisualState(PipeDirection connectedDirections) - { - ConnectedDirections = connectedDirections; - } - } - [Flags] [Serializable, NetSerializable] public enum PipeDirection diff --git a/Content.Shared/Botany/PlantHolderVisuals.cs b/Content.Shared/Botany/PlantHolderVisuals.cs index 04680f050b..b7dd0e6bc9 100644 --- a/Content.Shared/Botany/PlantHolderVisuals.cs +++ b/Content.Shared/Botany/PlantHolderVisuals.cs @@ -6,7 +6,8 @@ namespace Content.Shared.Botany [Serializable, NetSerializable] public enum PlantHolderVisuals { - Plant, + PlantRsi, + PlantState, HealthLight, WaterLight, NutritionLight, diff --git a/Content.Shared/Chemistry/SolutionVisuals.cs b/Content.Shared/Chemistry/SolutionVisuals.cs index 23d11e7f15..645a2d835f 100644 --- a/Content.Shared/Chemistry/SolutionVisuals.cs +++ b/Content.Shared/Chemistry/SolutionVisuals.cs @@ -11,7 +11,7 @@ namespace Content.Shared.Chemistry } [Serializable, NetSerializable] - public class SolutionContainerVisualState + public class SolutionContainerVisualState : ICloneable { public readonly Color Color; @@ -33,6 +33,11 @@ namespace Content.Shared.Chemistry Color = color; FilledVolumeFraction = (byte) (byte.MaxValue * filledVolumeFraction); } + + public object Clone() + { + return new SolutionContainerVisualState(Color, FilledVolumeFraction); + } } public enum SolutionContainerLayers : byte diff --git a/Content.Shared/Damage/DamageVisualizerKeys.cs b/Content.Shared/Damage/DamageVisualizerKeys.cs index dfaa598589..1a7abeca4e 100644 --- a/Content.Shared/Damage/DamageVisualizerKeys.cs +++ b/Content.Shared/Damage/DamageVisualizerKeys.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Robust.Shared.Serialization; namespace Content.Shared.Damage @@ -11,4 +12,20 @@ namespace Content.Shared.Damage DamageUpdateGroups, ForceUpdate } + + [Serializable, NetSerializable] + public class DamageVisualizerGroupData : ICloneable + { + public List GroupList; + + public DamageVisualizerGroupData(List groupList) + { + GroupList = groupList; + } + + public object Clone() + { + return new DamageVisualizerGroupData(new List(GroupList)); + } + } } diff --git a/Content.Shared/Damage/Systems/DamageableSystem.cs b/Content.Shared/Damage/Systems/DamageableSystem.cs index 6a84fe4cfc..c3f02a2085 100644 --- a/Content.Shared/Damage/Systems/DamageableSystem.cs +++ b/Content.Shared/Damage/Systems/DamageableSystem.cs @@ -87,7 +87,10 @@ namespace Content.Shared.Damage component.Dirty(); if (EntityManager.TryGetComponent(component.Owner, out var appearance) && damageDelta != null) - appearance.SetData(DamageVisualizerKeys.DamageUpdateGroups, damageDelta.GetDamagePerGroup().Keys.ToList()); + { + var data = new DamageVisualizerGroupData(damageDelta.GetDamagePerGroup().Keys.ToList()); + appearance.SetData(DamageVisualizerKeys.DamageUpdateGroups, data); + } RaiseLocalEvent(component.Owner, new DamageChangedEvent(component, damageDelta, interruptsDoAfters), false); } @@ -198,7 +201,7 @@ namespace Content.Shared.Damage component.DamageModifierSetId = state.ModifierSetId; // Has the damage actually changed? - DamageSpecifier newDamage = new() { DamageDict = state.DamageDict }; + DamageSpecifier newDamage = new() { DamageDict = new(state.DamageDict) }; var delta = component.Damage - newDamage; delta.TrimZeros(); diff --git a/Content.Shared/Hands/Components/SharedHandsComponent.cs b/Content.Shared/Hands/Components/SharedHandsComponent.cs index 3035bfccc7..202ba44d89 100644 --- a/Content.Shared/Hands/Components/SharedHandsComponent.cs +++ b/Content.Shared/Hands/Components/SharedHandsComponent.cs @@ -718,7 +718,7 @@ namespace Content.Shared.Hands.Components } [Serializable, NetSerializable] - public class HandsVisualState + public class HandsVisualState : ICloneable { public List Hands { get; } = new(); @@ -726,10 +726,15 @@ namespace Content.Shared.Hands.Components { Hands = hands; } + + public object Clone() + { + return new HandsVisualState(new List(Hands)); + } } [Serializable, NetSerializable] - public class HandVisualState + public struct HandVisualState { public string RsiPath { get; } public string? EquippedPrefix { get; } diff --git a/Content.Shared/StatusEffect/StatusEffectsSystem.cs b/Content.Shared/StatusEffect/StatusEffectsSystem.cs index b0982f91c8..3345507015 100644 --- a/Content.Shared/StatusEffect/StatusEffectsSystem.cs +++ b/Content.Shared/StatusEffect/StatusEffectsSystem.cs @@ -38,7 +38,7 @@ namespace Content.Shared.StatusEffect foreach (var state in status.ActiveEffects.ToArray()) { // if we're past the end point of the effect - if (_gameTiming.CurTime > state.Value.Cooldown.Item2) + if (curTime > state.Value.Cooldown.Item2) { TryRemoveStatusEffect(status.Owner, state.Key, status); } @@ -53,24 +53,33 @@ namespace Content.Shared.StatusEffect private void OnHandleState(EntityUid uid, StatusEffectsComponent component, ref ComponentHandleState args) { - if (args.Current is StatusEffectsComponentState state) + if (args.Current is not StatusEffectsComponentState state) + return; + + component.AllowedEffects = new(state.AllowedEffects); + + // Remove non-existent effects. + foreach (var effect in component.ActiveEffects.Keys) { - component.AllowedEffects = state.AllowedEffects; - - foreach (var effect in state.ActiveEffects) + if (!state.ActiveEffects.ContainsKey(effect)) { - // don't bother with anything if we already have it - if (component.ActiveEffects.ContainsKey(effect.Key)) - { - component.ActiveEffects[effect.Key] = effect.Value; - continue; - } - - var time = effect.Value.Cooldown.Item2 - effect.Value.Cooldown.Item1; - //TODO: Not sure how to handle refresh here. - TryAddStatusEffect(uid, effect.Key, time, true); + TryRemoveStatusEffect(uid, effect, component); } } + + foreach (var effect in state.ActiveEffects) + { + // don't bother with anything if we already have it + if (component.ActiveEffects.ContainsKey(effect.Key)) + { + component.ActiveEffects[effect.Key] = effect.Value; + continue; + } + + var time = effect.Value.Cooldown.Item2 - effect.Value.Cooldown.Item1; + //TODO: Not sure how to handle refresh here. + TryAddStatusEffect(uid, effect.Key, time, true); + } } /// @@ -140,7 +149,7 @@ namespace Content.Shared.StatusEffect /// /// This obviously does not add any actual 'effects' on its own. Use the generic overload, /// which takes in a component type, if you want to automatically add and remove a component. - /// + /// /// If the effect already exists, it will simply replace the cooldown with the new one given. /// If you want special 'effect merging' behavior, do it your own damn self! /// @@ -187,7 +196,7 @@ namespace Content.Shared.StatusEffect _alertsSystem.ShowAlert(uid, proto.Alert.Value, null, cooldown1); } - status.Dirty(); + Dirty(status); // event? return true; } @@ -261,7 +270,7 @@ namespace Content.Shared.StatusEffect status.ActiveEffects.Remove(key); - status.Dirty(); + Dirty(status); // event? return true; } @@ -285,6 +294,7 @@ namespace Content.Shared.StatusEffect failed = true; } + Dirty(status); return failed; } @@ -352,6 +362,7 @@ namespace Content.Shared.StatusEffect _alertsSystem.ShowAlert(uid, proto.Alert.Value, null, cooldown); } + Dirty(status); return true; } @@ -387,6 +398,7 @@ namespace Content.Shared.StatusEffect _alertsSystem.ShowAlert(uid, proto.Alert.Value, null, cooldown); } + Dirty(status); return true; } @@ -406,6 +418,8 @@ namespace Content.Shared.StatusEffect return false; status.ActiveEffects[key].Cooldown = (_gameTiming.CurTime, _gameTiming.CurTime + time); + + Dirty(status); return true; } diff --git a/Content.Shared/Storage/Components/SharedMapLayerData.cs b/Content.Shared/Storage/Components/SharedMapLayerData.cs index 9b1fad5148..5f9f82f8d8 100644 --- a/Content.Shared/Storage/Components/SharedMapLayerData.cs +++ b/Content.Shared/Storage/Components/SharedMapLayerData.cs @@ -24,7 +24,7 @@ namespace Content.Shared.Storage.Components } [Serializable, NetSerializable] - public class ShowLayerData + public class ShowLayerData : ICloneable { public IReadOnlyList QueuedEntities { get; internal set; } @@ -33,14 +33,14 @@ namespace Content.Shared.Storage.Components QueuedEntities = new List(); } - public ShowLayerData(IReadOnlyList other) + public ShowLayerData(IEnumerable other) { - QueuedEntities = other; + QueuedEntities = new List(other); } - public ShowLayerData(ShowLayerData other) + public object Clone() { - QueuedEntities = other.QueuedEntities; + return new ShowLayerData(QueuedEntities); } } }