Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UninitializedSaveTest prototypes #16144

Merged
merged 12 commits into from
May 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 16 additions & 59 deletions Content.IntegrationTests/Tests/PrototypeSaveTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,64 +38,6 @@ public sealed class PrototypeSaveTest
{
"Singularity", // physics collision uses "AllMask" (-1). The flag serializer currently fails to save this because this features un-named bits.
"constructionghost",

// These ones are from the serialization change to alwayswrite.
// These should NOT be added to.
// 99% of these are going to be changing the physics bodytype (where the entity is anchored)
// or some ambientsound change.
"GasVentScrubber",
"GasPassiveVent",
"CableHV",
"ParticleAcceleratorFuelChamberUnfinished",
"ComfyChair",
"PlasticFlapsOpaque",
"ParticleAcceleratorEmitterRightUnfinished",
"PlasticFlapsAirtightClear",
"SignalControlledValve",
"SignalControlledValve",
"GasPipeTJunction",
"GasFilter",
"GasOutletInjector",
"GasPressurePump",
"SurveillanceWirelessCameraAnchoredEntertainment",
"GasPort",
"Chair",
"GasMixer",
"ParticleAcceleratorPowerBoxUnfinished",
"GasValve",
"Thruster",
"BoxingBell",
"CableApcExtension",
"PlasticFlapsClear",
"ClothingBackpackChameleon",
"AMEControllerUnanchored",
"GasPipeFourway",
"NuclearBomb",
"PlasticFlapsAirtightOpaque",
"ParticleAcceleratorControlBoxUnfinished",
"GasPipeHalf",
"GasVolumePump",
"ParticleAcceleratorEmitterLeftUnfinished",
"GasMixerFlipped",
"ToiletDirtyWater",
"GasPipeBend",
"ParticleAcceleratorEndCapUnfinished",
"GasPipeStraight",
"MachineFrameDestroyed",
"ChairPilotSeat",
"VehicleJanicartDestroyed",
"Gyroscope",
"ParticleAcceleratorEmitterCenterUnfinished",
"ToiletEmpty",
"GasPassiveGate",
"CableMV",
"ClothingBackpackChameleonFill",
"GasDualPortVentPump",
"GasVentPump",
"PressureControlledValve",
"GasFilterFlipped",
"SurveillanceWirelessCameraAnchoredConstructed",

};

[Test]
Expand Down Expand Up @@ -127,7 +69,7 @@ await server.WaitPost(() =>

grid = mapManager.CreateGrid(mapId);

var tileDefinition = tileDefinitionManager["UnderPlating"];
var tileDefinition = tileDefinitionManager["FloorSteel"]; // Wires n such disable ambiance while under the floor
var tile = new Tile(tileDefinition.TileId);
var coordinates = grid.ToCoordinates();

Expand Down Expand Up @@ -168,6 +110,7 @@ await server.WaitAssertion(() =>
foreach (var prototype in prototypes)
{
uid = entityMan.SpawnEntity(prototype.ID, testLocation);
context.Prototype = prototype;

// get default prototype data
Dictionary<string, MappingDataNode> protoData = new();
Expand All @@ -177,9 +120,11 @@ await server.WaitAssertion(() =>

foreach (var (compType, comp) in prototype.Components)
{
context.WritingComponent = compType;
protoData.Add(compType, seriMan.WriteValueAs<MappingDataNode>(comp.Component.GetType(), comp.Component, alwaysWrite: true, context: context));
}

context.WritingComponent = string.Empty;
context.WritingReadingPrototypes = false;
}
catch (Exception e)
Expand All @@ -202,6 +147,7 @@ await server.WaitAssertion(() =>
MappingDataNode compMapping;
try
{
context.WritingComponent = compName;
compMapping = seriMan.WriteValueAs<MappingDataNode>(compType, component, alwaysWrite: true, context: context);
}
catch (Exception e)
Expand Down Expand Up @@ -246,6 +192,9 @@ private sealed class TestEntityUidContext : ISerializationContext,
public SerializationManager.SerializerProvider SerializerProvider { get; }
public bool WritingReadingPrototypes { get; set; }

public string WritingComponent = string.Empty;
public EntityPrototype Prototype = default!;

public TestEntityUidContext()
{
SerializerProvider = new();
Expand All @@ -262,6 +211,14 @@ public DataNode Write(ISerializationManager serializationManager, EntityUid valu
IDependencyCollection dependencies, bool alwaysWrite = false,
ISerializationContext? context = null)
{
if (WritingComponent != "Transform" && !Prototype.NoSpawn)
{
// Maybe this will be necessary in the future, but at the moment it just indicates that there is some
// issue, like a non-nullable entityUid data-field. If a component MUST have an entity uid to work with,
// then the prototype very likely has to be a no-spawn entity that is never meant to be directly spawned.
Assert.Fail($"Uninitialized entities should not be saving entity Uids. Component: {WritingComponent}. Prototype: {Prototype.ID}");
}

return new ValueDataNode(value.ToString());
}

Expand Down
2 changes: 1 addition & 1 deletion Content.Server/Dragon/Components/DragonRiftComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public sealed class DragonRiftComponent : SharedDragonRiftComponent
/// <summary>
/// Dragon that spawned this rift.
/// </summary>
[DataField("dragon")] public EntityUid Dragon;
[DataField("dragon")] public EntityUid? Dragon;

/// <summary>
/// How long the rift has been active.
Expand Down
4 changes: 2 additions & 2 deletions Content.Server/Dragon/DragonSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ private void OnRiftShutdown(EntityUid uid, DragonRiftComponent component, Compon

// We can't predict the rift being destroyed anyway so no point adding weakened to shared.
dragon.WeakenedAccumulator = dragon.WeakenedDuration;
_movement.RefreshMovementSpeedModifiers(component.Dragon);
_popupSystem.PopupEntity(Loc.GetString("carp-rift-destroyed"), component.Dragon, component.Dragon);
_movement.RefreshMovementSpeedModifiers(component.Dragon.Value);
_popupSystem.PopupEntity(Loc.GetString("carp-rift-destroyed"), component.Dragon.Value, component.Dragon.Value);
}
}

Expand Down
34 changes: 17 additions & 17 deletions Content.Server/Lightning/LightningSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,28 @@ private void OnRemove(EntityUid uid, LightningComponent component, ComponentRemo

private void OnCollide(EntityUid uid, LightningComponent component, ref StartCollideEvent args)
{
if (!TryComp<BeamComponent>(uid, out var lightningBeam) || !TryComp<BeamComponent>(lightningBeam.VirtualBeamController, out var beamController))
Comment on lines 38 to -40
Copy link
Member Author

@ElectroJr ElectroJr May 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this function around while making ArcTarget nullable.

I'm not entirely sure what the function was meant to be doing, but AFAICT there was a chance it would enter an infinite loop if Arc failed to find a target? So now it no longer does a while loop, which might change some behaviour if it fails to find a target during the first attempt.

{
if (!TryComp<BeamComponent>(uid, out var lightningBeam)
|| !TryComp<BeamComponent>(lightningBeam.VirtualBeamController, out var beamController))
return;
}

if (component.CanArc)
{
while (beamController.CreatedBeams.Count < component.MaxTotalArcs)
{
Arc(component, args.OtherFixture.Body.Owner, lightningBeam.VirtualBeamController.Value);

var spriteState = LightningRandomizer();
if (!component.CanArc && beamController.CreatedBeams.Count < component.MaxTotalArcs)
return;

component.ArcTargets.Add(args.OtherFixture.Body.Owner);
component.ArcTargets.Add(component.ArcTarget);
Arc(component, args.OtherEntity, lightningBeam.VirtualBeamController.Value);

_beam.TryCreateBeam(args.OtherFixture.Body.Owner, component.ArcTarget, component.LightningPrototype, spriteState, controller: lightningBeam.VirtualBeamController.Value);
if (component.ArcTarget == null)
return;

//Break from this loop so other created bolts can collide and arc
break;
}
}
var spriteState = LightningRandomizer();
component.ArcTargets.Add(args.OtherEntity);
component.ArcTargets.Add(component.ArcTarget.Value);

_beam.TryCreateBeam(
args.OtherEntity,
component.ArcTarget.Value,
component.LightningPrototype,
spriteState,
controller: lightningBeam.VirtualBeamController.Value);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public abstract class SharedLightningComponent : Component
/// The target that the lightning will Arc to.
/// </summary>
[DataField("arcTarget")]
public EntityUid ArcTarget;
public EntityUid? ArcTarget;

/// <summary>
/// How far should this lightning go?
Expand Down
3 changes: 0 additions & 3 deletions Resources/Prototypes/Entities/Clothing/Back/specific.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
tags: [] # ignore "WhitelistChameleon" tag
- type: Sprite
sprite: Clothing/Back/Backpacks/backpack.rsi
netsync: false
- type: Clothing
sprite: Clothing/Back/Backpacks/backpack.rsi
- type: ChameleonClothing
slot: [back]
default: ClothingBackpack
Expand Down
5 changes: 5 additions & 0 deletions Resources/Prototypes/Entities/Effects/lightning.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
name: lightning
id: Lightning
parent: BaseLightning
noSpawn: true
components:
- type: Lightning
canArc: true
Expand All @@ -42,6 +43,7 @@
name: spooky lightning
id: LightningRevenant
parent: BaseLightning
noSpawn: true
components:
- type: Sprite
sprite: /Textures/Effects/lightning.rsi
Expand All @@ -65,6 +67,7 @@
name: charged lightning
id: ChargedLightning
parent: BaseLightning
noSpawn: true
components:
- type: Sprite
sprite: /Textures/Effects/lightning.rsi
Expand All @@ -83,6 +86,7 @@
name: supercharged lightning
id: SuperchargedLightning
parent: ChargedLightning
noSpawn: true
components:
- type: Sprite
sprite: /Textures/Effects/lightning.rsi
Expand All @@ -108,6 +112,7 @@
name: hypercharged lightning
id: HyperchargedLightning
parent: ChargedLightning
noSpawn: true
components:
- type: Sprite
sprite: /Textures/Effects/lightning.rsi
Expand Down
2 changes: 1 addition & 1 deletion Resources/Prototypes/Entities/Objects/Devices/nuke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
noRot: true
state: nuclearbomb_base
- type: Physics
bodyType: Dynamic
bodyType: Static
- type: Fixtures
fixtures:
- shape:
Expand Down
6 changes: 6 additions & 0 deletions Resources/Prototypes/Entities/Structures/Furniture/chairs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
components:
- type: Transform
anchored: true
- type: Physics
bodyType: Static
- type: Anchorable
- type: Rotatable
- type: Sprite
Expand Down Expand Up @@ -127,6 +129,8 @@
components:
- type: Transform
anchored: true
- type: Physics
bodyType: Static
- type: Anchorable
- type: Rotatable
- type: Sprite
Expand All @@ -143,6 +147,8 @@
components:
- type: Transform
anchored: true
- type: Physics
bodyType: Static
- type: Anchorable
- type: Rotatable
- type: Sprite
Expand Down
2 changes: 2 additions & 0 deletions Resources/Prototypes/Entities/Structures/Furniture/toilet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
maxVol: 250
- type: Transform
anchored: true
- type: Physics
bodyType: Static
- type: Construction
graph: Toilet
node: toilet
Expand Down
2 changes: 2 additions & 0 deletions Resources/Prototypes/Entities/Structures/Machines/frame.yml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@
- type: Transform
anchored: true
noRot: true
- type: Physics
bodyType: Static
- type: Construction
graph: Machine
node: destroyedMachineFrame
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
- type: Anchorable
- type: Transform
anchored: true
- type: Physics
bodyType: Static
- type: Sprite
noRot: true
sprite: Structures/monitors.rsi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
graph: GasBinary
node: valve
- type: AmbientSound
enabled: false
enabled: true
volume: -9
range: 5
sound:
Expand Down Expand Up @@ -229,7 +229,7 @@
graph: GasBinary
node: signalvalve
- type: AmbientSound
enabled: false
enabled: true
volume: -9
range: 5
sound:
Expand Down Expand Up @@ -313,6 +313,8 @@
!type:PipeNode
nodeGroupID: Pipe
pipeDirection: South
- type: AmbientSound
enabled: true

- type: entity
parent: [ BaseMachine, ConstructibleMachine ]
Expand Down Expand Up @@ -355,7 +357,7 @@
- type: AtmosDevice
- type: AtmosPipeColor
- type: AmbientSound
enabled: false
enabled: true
volume: -9
range: 5
sound:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- Pipe
- type: Physics
canCollide: false
bodyType: static
- type: AmbientSound
enabled: false
volume: -15
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
node: ventpump
- type: VentCritterSpawnLocation
- type: AmbientSound
enabled: false
enabled: true
volume: -12
range: 5
sound:
Expand Down Expand Up @@ -171,7 +171,7 @@
graph: GasUnary
node: ventscrubber
- type: AmbientSound
enabled: false
enabled: true
volume: -12
range: 5
sound:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@
suffix: Unfinished
description: This controls the density of the particles. It looks unfinished.
components:
- type: Physics
bodyType: Dynamic
- type: Sprite
sprite: Structures/Power/Generation/PA/control_box.rsi
- type: Construction
Expand Down
Loading