Add Multiple Tracking Options to the Pinpointer #418

Spatison · 2024-05-23T21:42:44Z

Description

PR adds a system of multiple targets for one pinpointer.
Reworks the pinpointer for nuclear operatives, adding the ability to track a nuclear sindicat bomb and a syndicate shuttle along with the nuclear authentication disk.
Nuclear bomb in Shuttle Syndicate now always comes with its codes.

TODO

Add multiple Pinpointer

Media

https://youtu.be/YCcFzrtYp8E

Changelog

🆑

add: Multiple pinpointer
add: Syndicat nuclear bomb
fix: Sindicat to Syndicate
delete: Syndicat Nuclear Code
change: The station's nuclear code is now connected to any bomb on the same map

DangerRevolution · 2024-05-23T21:51:03Z

Description

my first PR. I do not know what to write here.

What does this PR do?

FoxxoTrystan · 2024-05-23T22:28:09Z

Can you edit this PR description and give more details?

What exactly is it? why is it needed, ect...

DEATHB4DEFEAT · 2024-05-23T22:45:38Z

Media: https://drive.google.com/file/d/1UyXTHHVVUx7ZAvHG4_U6_ncnWWrn4FyE/view?usp=drivesdk

It would be preferable for both ease and history to directly embed it in the PR description.
If you can't reasonably compress it to fit 8MB I'd recommend putting it on YouTube instead of Google Drive so it's easier to keep longer and you get free views on your channel.
No matter what option you pick at least put media in the media section of the PR description.

Pspritechologist

Overall I'd like to see the solution made less hard-coded. Right now it requires modifying an enum to add new modes to a pinpointer- ideally a 'mode' would be a serializable object, allowing you to define the properties of that mode entirely in YAML.

Pspritechologist · 2024-05-23T23:27:20Z

Content.Client/Pinpointer/PinpointerSystem.cs

Please avoid modifying files that aren't relevant. You should be able to simply 'revert changes' on this file without affecting anything.

Pspritechologist · 2024-05-23T23:28:47Z

Content.Shared/Localizations/ContentLocalizationManager.cs

This is certainly not relevant to this pr :P

Pspritechologist · 2024-05-23T23:30:04Z

Content.Shared/Nuke/NuclearBombSindicate.cs

+/// Used for tracking the nuke bomb - isn't a tag for pinpointer purposes.
+/// </summary>
+[RegisterComponent, NetworkedComponent]
+public sealed partial class NuclearBombSindicateComponent : Component


This isn't how you spell Syndicate.

This.

I assume this the closest English spelling of Синдикат

Pspritechologist · 2024-05-23T23:30:15Z

Content.Shared/Nuke/SharedNuke.cs

+    {
+        ALL,
+        STATION,
+        SINDICAT


This is even less how you spell Syndicate

Pspritechologist · 2024-05-23T23:31:24Z

Content.Shared/Nuke/SharedNuke.cs

+    public enum NukesAvailable : byte
+    {
+        ALL,
+        STATION,
+        SINDICAT
+    }


The solution of having an Enum for explicit 'modes' of nuke tracking being 'All', 'Station', and 'Syndicate' is a very hardcoded solution to what should be entirely doable in YAML. I feel for this pr to make sense the Pinpointer itself just needs to be slightly reworked.

Pspritechologist · 2024-05-23T23:37:36Z

Content.Server/Pinpointer/PinpointerSystem.multiple.cs

+
+    private void OnMultiplePinpointerStartup(EntityUid uid, MultiplePinpointerComponent multiple, ComponentStartup args)
+    {
+        if (EntityManager.TryGetComponent(uid, out PinpointerComponent? tool))


This should probably just be an EnsureComp rather than not doing anything if it isn't found.

Pspritechologist · 2024-05-23T23:38:48Z

Content.Server/Pinpointer/PinpointerSystem.multiple.cs

The public methods here should have XML docs.

Pspritechologist · 2024-05-23T23:39:26Z

Content.Server/Pinpointer/PinpointerSystem.multiple.cs

+        if (!Resolve(uid, ref multiple, ref pin))
+            return;
+
+        Dirty(uid, multiple);


Is there a reason we're dirtying it right at the start?

Pspritechologist · 2024-05-23T23:41:17Z

Content.Server/Pinpointer/PinpointerSystem.multiple.cs

+        var modes = new string[multiple.Modes.Count];
+        var i = 0;
+
+        foreach (var tag in multiple.Modes)
+        {
+            modes[i] = tag;
+            i++;
+        }
+
+        var current = modes[multiple.CurrentEntry];


Am I crazy or does this not make any sense?

Pspritechologist · 2024-05-23T23:45:45Z

Content.Server/Pinpointer/PinpointerSystem.cs

Seems to be some unneeded changes in this file

DangerRevolution

I'm repeating a lot of what PSprite said but this is not a flexible system. The system, ideally, should track any entity with specific tags or components, defined in .yml.

As an example, I should be able to make a pinpointer that tracks the closest chair with a tag or component of "wooden".

I should be able to define in .yml what the pinpointer should track, is the point.

DangerRevolution · 2024-05-24T00:19:04Z

Content.Client/Pinpointer/PinpointerSystem.cs

This does nothing. Refer to above.

DangerRevolution · 2024-05-24T00:19:29Z

Content.Server/Pinpointer/PinpointerSystem.cs

DangerRevolution · 2024-05-24T00:19:47Z

Content.Server/Nuke/NukeCodePaperSystem.cs

-                    nuke.OriginStation != owningStation))
+                     nuke.OriginMapGrid != (transform.MapID, transform.GridUid) ||
+                     nuke.OriginStation != owningStation)) ||
+                    (NukesAvailable == NukesAvailable.SINDICAT &&


Suggested change

(NukesAvailable == NukesAvailable.SINDICAT &&

(NukesAvailable == NukesAvailable.SYNDICATE &&

DangerRevolution · 2024-05-24T00:20:32Z

Content.Shared/Localizations/ContentLocalizationManager.cs

DangerRevolution · 2024-05-24T00:21:02Z

Content.Shared/Nuke/NuclearBombSindicate.cs

+/// Used for tracking the nuke bomb - isn't a tag for pinpointer purposes.
+/// </summary>
+[RegisterComponent, NetworkedComponent]
+public sealed partial class NuclearBombSindicateComponent : Component


This.

I assume this the closest English spelling of Синдикат

DangerRevolution · 2024-05-24T00:24:44Z

Resources/Prototypes/Entities/Objects/Devices/pinpointer.yml

+  name: syndicate pinpointer
+  description: Produced specifically for nuclear operative missions, get that disk!
+  id: PinpointerLonolyOperative
+  suffix: LonolyOps


I don't like this suffix name.

DangerRevolution · 2024-05-24T00:25:07Z

Resources/Prototypes/Entities/Objects/Misc/paper.yml

+  suffix: Sindicat
+  components:
+  - type: NukeCodePaper
+    NukesAvailable: Sindicat


Suggested change

NukesAvailable: Sindicat

NukesAvailable: Syndicate

DangerRevolution · 2024-05-24T00:25:17Z

Resources/Prototypes/Entities/Objects/Misc/paper.yml

+- type: entity
+  parent: NukeCodePaper
+  id: NukeCodePaperSindicat
+  suffix: Sindicat


Suggested change

suffix: Sindicat

suffix: Syndicate

DangerRevolution · 2024-05-24T00:25:24Z

Resources/Prototypes/Entities/Objects/Misc/paper.yml

+
+- type: entity
+  parent: NukeCodePaper
+  id: NukeCodePaperSindicat


Suggested change

id: NukeCodePaperSindicat

id: NukeCodePaperSyndicate

DangerRevolution · 2024-05-24T00:26:23Z

Resources/Prototypes/Roles/Jobs/Fun/misc_startinggear.yml

@@ -114,6 +114,26 @@
  satchel: ClothingBackpackDuffelSyndicateOperative
  duffelbag: ClothingBackpackDuffelSyndicateOperative

+#Syndicate Lonoly Operative Outfit - Full Kit


Why is this a full gear change from the regular operatives? To include the pinpointer?

Spatison · 2024-05-30T20:59:45Z

So, how can i fix the checks?

VMSolidus · 2024-05-30T21:00:47Z

So, how can i fix the checks?

I got it for you, the tests should run on your PR now. It just required a merge master to get a key update to make the tests work again after Microsoft broke them.

DangerRevolution · 2024-06-20T22:56:36Z

Going to set this as draft; issues marked by PSprite are still not addressed

misandrie · 2024-06-21T15:20:13Z

Resources/Prototypes/Roles/Jobs/Fun/misc_startinggear.yml

+#Syndicate Lonoly Operative Outfit - include a special pinpointer
+- type: startingGear
+  id: SyndicateLonolyOperativeGearFull


misandrie · 2024-06-21T15:20:26Z

Resources/Maps/Shuttles/infiltrator.yml

    components:
    - type: Transform
      pos: -2.5,-12.5
      parent: 1
- proto: NukeCodePaper
+- proto: NukeCodePaperStation


This is done so that the nuclear bomb in Shuttle Syndicate is always provided with its own codes.

misandrie · 2024-06-21T15:21:09Z

Resources/Prototypes/Catalog/Fills/Backpacks/duffelbag.yml

@@ -332,6 +332,18 @@
      - id: PinpointerSyndicateNuclear
      - id: DeathAcidifierImplanter

+- type: entity
+  parent: ClothingBackpackDuffelSyndicateBundle
+  id: ClothingBackpackDuffelSyndicateLonolyOperative


misandrie · 2024-06-21T15:21:16Z

Resources/Prototypes/Entities/Mobs/Player/human.yml

@@ -62,7 +62,7 @@
      randomizeName: false
    - type: NukeOperative
    - type: Loadout
-      prototypes: [SyndicateOperativeGearFull]
+      prototypes: [SyndicateLonolyOperativeGearFull]


misandrie · 2024-06-21T15:22:34Z

Resources/Prototypes/Roles/Jobs/Fun/misc_startinggear.yml

+  satchel: ClothingBackpackDuffelSyndicateLonolyOperative
+  duffelbag: ClothingBackpackDuffelSyndicateLonolyOperative


Do these exist?

…n-Engines into NukeSmallRevork

github-actions bot added Changes: C# Changes any cs files Changes: Map Changes any yml file in the Maps directories labels May 23, 2024

DangerRevolution requested review from VMSolidus, SimpleStation14 and DangerRevolution May 23, 2024 22:59

github-actions bot added the Status: Needs Review Someone please review this label May 23, 2024

DangerRevolution added Changes: YML Changes any yml files Priority: 4-Low Should be resolved at some point Size: 3-Medium For medium issues/PRs Status: Needs Cleanup Someone has to clean this before merging Type: Feature Creation of or significant changes to a feature labels May 23, 2024

Pspritechologist requested changes May 23, 2024

View reviewed changes

DangerRevolution changed the title ~~add Multiple Pinpointer~~ Add multiple tracking options to the pinpointer May 24, 2024

DangerRevolution requested changes May 24, 2024

View reviewed changes

commit

5b4c99c

Spatison force-pushed the NukeSmallRevork branch from 548ac96 to 5b4c99c Compare May 24, 2024 22:45

github-actions bot removed the Changes: YML Changes any yml files label May 24, 2024

fix

b18f3ab

fix

0ffca73

Spatison force-pushed the NukeSmallRevork branch from 918db90 to 0ffca73 Compare May 31, 2024 14:06

Merge branch 'master' into NukeSmallRevork

87cdf5c

Spatison requested a review from Pspritechologist June 1, 2024 09:13

DangerRevolution marked this pull request as draft June 20, 2024 22:56

EnsureComp

f56b945

SimpleStation14 changed the title ~~Add multiple tracking options to the pinpointer~~ Add Multiple Tracking Options to the Pinpointer Jun 21, 2024

Merge branch 'master' into NukeSmallRevork

2cf24ef

misandrie reviewed Jun 21, 2024

View reviewed changes

Spatison added 2 commits June 21, 2024 18:47

correction of typos

1e4b977

Merge branch 'NukeSmallRevork' of https://github.com/Spatison/Einstei…

f3bb3b3

…n-Engines into NukeSmallRevork

DangerRevolution assigned Spatison Jun 22, 2024

	(NukesAvailable == NukesAvailable.SINDICAT &&
	(NukesAvailable == NukesAvailable.SYNDICATE &&

		satchel: ClothingBackpackDuffelSyndicateLonolyOperative
		duffelbag: ClothingBackpackDuffelSyndicateLonolyOperative

Add Multiple Tracking Options to the Pinpointer #418

Are you sure you want to change the base?

Add Multiple Tracking Options to the Pinpointer #418

Conversation

Spatison commented May 23, 2024 • edited Loading

Description

TODO

Media

Changelog

DangerRevolution commented May 23, 2024

Description

FoxxoTrystan commented May 23, 2024

DEATHB4DEFEAT commented May 23, 2024

Pspritechologist 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

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

DangerRevolution 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

Choose a reason for hiding this comment

Spatison commented May 30, 2024

VMSolidus commented May 30, 2024

DangerRevolution commented Jun 20, 2024

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

Spatison commented May 23, 2024 •

edited

Loading