NVG (Port From _LostParadise) #1128

BL02DL · 2024-10-22T14:09:03Z

Description

The influence on the game is manifested in the fact that these are inherently improved visors of various departments with the ability to see in the dark, this is also necessary for the future cult of Narsi.

This NVG has no analogues at the moment and is one of the best variations of it in the game.

As I have already said, this night vision device is necessary in the code plan for the implementation of one of the objects of the cult

TODO

This PR is already complete.

Changelog

🆑 BL02DL

add: Added NVG!

github-actions · 2024-10-22T14:09:39Z

RSI Diff Bot; head commit 76344ab merging into 85a770c
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Clothing/Eyes/NVG/diagnosticnightvision.rsi

State	Old	New	Status
equipped-EYES			Added
icon-on			Added
icon			Added
inhand-left			Added
inhand-right			Added
on-equipped-EYES			Added
on-inhand-left			Added
on-inhand-right			Added

Resources/Textures/Clothing/Eyes/NVG/mednightvision.rsi

State	Old	New	Status
equipped-EYES			Added
icon-on			Added
icon			Added
inhand-left			Added
inhand-right			Added
on-equipped-EYES			Added
on-inhand-left			Added
on-inhand-right			Added

Resources/Textures/Clothing/Eyes/NVG/nightvision.rsi

State	Old	New	Status
equipped-EYES			Added
icon-on			Added
icon			Added
inhand-left			Added
inhand-right			Added
on-equipped-EYES			Added
on-inhand-left			Added
on-inhand-right			Added
toggle-off			Added
toggle-on			Added

Resources/Textures/Clothing/Eyes/NVG/secnightvision.rsi

State	Old	New	Status
equipped-EYES			Added
icon-on			Added
icon			Added
inhand-left			Added
inhand-right			Added
on-equipped-EYES			Added
on-inhand-left			Added
on-inhand-right			Added

Edit: diff updated after 76344ab

Eagle-0 · 2024-10-22T14:33:04Z

I would wait and port NVG's from WD instead, they have thermals too

BL02DL · 2024-10-22T14:44:49Z

@Eagle-0 They were going to port their old version, which would be worse than mine. There is no toggle button and Alert. There is no color setting.

FoxxoTrystan

All of the code is simple and good, good job.

The only changes i will request is to REMOVE all additional namespaces, we do not need "_LostParadise", remember we are an upstream not a downstream and those does not help us, please move it.

Russian FTL seem be missing?

What does "LPP" Means? i seen it everywhere and looks wired.

Some items names/copyright details need to be better.

Minor code suggestion but again, good job!

Content.Client/IoC/ClientContentIoC.cs

Content.Client/_LostParadise/Clothing/NightVisionOverlay.cs

Content.Server/_LostParadise/Clothing/NightVisionSystem.cs

Resources/Prototypes/_LostParadise/Shaders/shaders.yml

Resources/Textures/_LostParadise/Clothing/Eyes/NVG/diagnosticnightvision.rsi/meta.json

Resources/Textures/_LostParadise/Clothing/Eyes/NVG/mednightvision.rsi/meta.json

Resources/Textures/_LostParadise/Clothing/Eyes/NVG/nightvision.rsi/meta.json

Resources/Textures/_LostParadise/Clothing/Eyes/NVG/secnightvision.rsi/meta.json

FoxxoTrystan · 2024-10-22T15:45:41Z

Can you also provide a "Media", how the nightvsion looks like when on? at least the default.

Remuchi

Haven't seen it in the game yet but what is the point of NVG alert? You can clearly understand that you have NVG enabled by the look of your... welp... game screen? IMO at the moment it just looks like excessive information and alert bloat (which we already have too much).
Maybe you should at least make it able to toggle NVG on/off. But I'd rather remove it.

Content.Client/IoC/ClientContentIoC.cs

Content.Client/_LostParadise/Clothing/NightVisionSystem.cs

Content.Client/_LostParadise/Clothing/NightVisionOverlay.cs

Content.Client/_LostParadise/Clothing/NightVisionSystem.cs

Content.Server/_LostParadise/Clothing/NightVisionSystem.cs

Content.Shared/_LostParadise/Clothing/NightVisionComponent.cs

BL02DL · 2024-10-22T16:07:26Z

@FoxxoTrystan Hi. Unfortunately, I can't attach any video to demonstrate NVG's work, I can only try screenshots due to the fact that I didn't do it on my work computer. And it looks like I got the translation file mixed up..

VMSolidus

This is only a cursory check. The only thing that stood out while I was on the github web was the use of magic strings hardcoding it to only work on the Eyes slot.

Content.Shared/Clothing/NightVisionComponent.cs

Content.Server/Clothing/NightVisionSystem.cs

Content.Client/Clothing/NightVisionSystem.cs

VMSolidus · 2024-10-23T04:27:56Z

Content.Client/Clothing/NightVisionSystem.cs

+    {
+        var playerUid = EntityUid.Parse(_playerManager.LocalPlayer?.ControlledEntity.ToString());
+        var slot = _entityManager.GetComponent<InventorySlotsComponent>(playerUid);
+        _entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);


Suggested change

_entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);

_entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);

Oh this is hardcoded in the "eyes" slot here too. This is incredibly cursed. You shouldn't need a "magic string" here to check only the eyes slot. What if I wanted to make a mech that gives you night vision, or a hardsuit helmet with built-in nightvision?

@VMSolidus What i need to do to fix this?

Content.Shared/Clothing/NightVisionComponent.cs

Content.Shared/Clothing/SharedNightVisionSystem.cs

Resources/Prototypes/Entities/Clothing/Eyes/NightVision.yml

Remuchi

Mostly fine. After resolving Raislin review and my commentaries can be merged.

Remuchi · 2024-10-23T14:46:09Z

Resources/Prototypes/Entities/Clothing/Eyes/NightVision.yml

+- type: entity
+  id: ActionBaseToggleNightVision
+  name: Toggle NVG
+  description: Toggles the NVG on and off.
+  noSpawn: true
+  components:
+  - type: InstantAction
+    itemIconStyle: NoItem
+    event: !type:ToggleNightVisionEvent
+


I really see no reason for it to exist. It doesn't have anything abstract or generic to be separated in abstract entity nor it's used somewhere in the code. It can be removed entirely.

Remuchi · 2024-10-23T14:47:05Z

Resources/Locale/en-US/NightVision/NightVision.ftl

+    .desc = { ent-ActionBaseToggleNightVision }
+ent-ClothingEyesNVG = night vision goggles
+    .desc = Now you can see in the dark! It has the label "BL CORP technology".
+toggle-nightvision-verb-get-data-text = Toggle Night Vision Goggles


I'd rather have it placed after/before all of the ent fields for the ease of reading.

Remuchi · 2024-10-23T14:47:10Z

Resources/Locale/ru-RU/NightVision/NightVision.ftl

+    .desc = { ent-ActionBaseToggleNightVision }
+ent-ClothingEyesNVG = ПНВ
+    .desc = Теперь ты можешь видеть в темноте! Имеет этикетку "BL CORP technology".
+toggle-nightvision-verb-get-data-text = Переключить прибор ночного видения


I'd rather have it placed after/before all of the ent fields for the ease of reading.

Remuchi · 2024-10-23T14:50:59Z

Content.Shared/Clothing/SharedNightVisionSystem.cs

+        nightvision.Enabled = !nightvision.Enabled;
+
+        if (_sharedContainer.TryGetContainingContainer(uid, out var container) &&
+            _inventory.TryGetSlotEntity(container.Owner, "eyes", out var entityUid) && entityUid == uid)


Suggested change

_inventory.TryGetSlotEntity(container.Owner, "eyes", out var entityUid) && entityUid == uid)

_inventory.TryGetSlotEntity(container.Owner, nightvision.Slot, out var entityUid) && entityUid == uid)

Remuchi · 2024-10-23T14:53:45Z

Also build fail not related to PR

Mnemotechnician

This is just bad.

Mnemotechnician · 2024-10-25T03:41:05Z

Content.Client/Clothing/NightVisionSystem.cs

+    [Dependency] private readonly IOverlayManager _overlayMan = default!;
+    [Dependency] private readonly ILightManager _lightManager = default!;
+    [Dependency] private readonly IPlayerManager _playerManager = default!;
+    [Dependency] private readonly IEntityManager _entityManager = default!;


Suggested change

[Dependency] private readonly IEntityManager _entityManager = default!;

Mnemotechnician · 2024-10-25T03:44:57Z

Content.Client/Clothing/NightVisionSystem.cs

+        var playerUid = EntityUid.Parse(_playerManager.LocalPlayer?.ControlledEntity.ToString());
+        var slot = _entityManager.GetComponent<InventorySlotsComponent>(playerUid);
+        _entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);


Why in the name of all that's unholy does this convert an entity uid to a string and parse it back?!

Below is a better example. It should work, but it was made in webedit and as such may contain a mistake/mistype/whatever.

Suggested change

var playerUid = EntityUid.Parse(_playerManager.LocalPlayer?.ControlledEntity.ToString());

var slot = _entityManager.GetComponent<InventorySlotsComponent>(playerUid);

_entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);

var playerUid = _playerManager.LocalPlayer?.ControlledEntity;

if (!TryComp<InventorySlotsComponent>(playerUid, out var slot) || !TryComp<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision))

return null;

Mnemotechnician · 2024-10-25T03:55:13Z

Content.Server/Clothing/NightVisionSystem.cs

+    private void OnGotUnequipped(EntityUid uid, NightVisionComponent component, GotUnequippedEvent args)
+    {
+        if (args.Slot == component.Slot)
+            UpdateNightVisionEffects(args.Equipee, uid, false, component);
+    }
+
+    private void OnGotEquipped(EntityUid uid, NightVisionComponent component, GotEquippedEvent args)
+    {
+        if (args.Slot == component.Slot)
+            UpdateNightVisionEffects(args.Equipee, uid, true, component);
+    }


This needs to account for the case where two or more clothings of the same type grant night vision (see solidus' comment about night vision helmets, etc)

Mnemotechnician · 2024-10-25T03:57:52Z

Content.Client/Clothing/NightVisionSystem.cs

+    private void OnGotUnequipped(EntityUid uid, NightVisionComponent component, GotUnequippedEvent args)
+    {
+        if (args.Slot == component.Slot)
+        {
+            _overlayMan.RemoveOverlay(_overlay);
+            _lightManager.DrawLighting = true;
+        }
+    }


Again, this needs to account for the case where two or more clothing types grant night vision at the same time.

NVG (Port from _LostParadise)

1a2992c

github-actions bot added Changes: C# Changes any cs files Changes: Localization Changes any ftl files Changes: YML Changes any yml files Changes: Sprite Changes any png or json in an RSI labels Oct 22, 2024

SimpleStation14 changed the title ~~NVG (Port from _LostParadise)~~ NVG (Port From _LostParadise) Oct 22, 2024

Update copyright

f5960c3

FoxxoTrystan requested review from a team, VMSolidus, FoxxoTrystan, DEATHB4DEFEAT, Peptide90, Pspritechologist and OldDanceJacket and removed request for a team October 22, 2024 15:28

FoxxoTrystan assigned BL02DL Oct 22, 2024

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

FoxxoTrystan requested changes Oct 22, 2024

View reviewed changes

FoxxoTrystan removed the Status: Needs Review Someone please review this label Oct 22, 2024

FoxxoTrystan added the Status: Awaiting Changes Do not merge due to requested changes label Oct 22, 2024

Remuchi requested changes Oct 22, 2024

View reviewed changes

BL02DL added 4 commits October 23, 2024 00:23

Requested changes

94f4454

Fix Copyright

12eb886

Requested changes

9bd2e91

fix ftl

250c0a9

FoxxoTrystan requested review from FoxxoTrystan and Remuchi October 22, 2024 20:05

github-actions bot added Status: Needs Review Someone please review this and removed Status: Awaiting Changes Do not merge due to requested changes labels Oct 22, 2024

Merge branch 'Simple-Station:master' into BL-CORP-NVG

61f68d0

VMSolidus requested changes Oct 23, 2024

View reviewed changes

Requested changes

76344ab

Remuchi reviewed Oct 23, 2024

View reviewed changes

Mnemotechnician reviewed Oct 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NVG (Port From _LostParadise) #1128

NVG (Port From _LostParadise) #1128

BL02DL commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024 •

edited

Loading

Eagle-0 commented Oct 22, 2024

BL02DL commented Oct 22, 2024

FoxxoTrystan left a comment

FoxxoTrystan commented Oct 22, 2024

Remuchi left a comment •

edited

Loading

BL02DL commented Oct 22, 2024

VMSolidus left a comment

VMSolidus Oct 23, 2024

BL02DL Oct 23, 2024

Remuchi left a comment

Remuchi Oct 23, 2024

Remuchi Oct 23, 2024

Remuchi Oct 23, 2024

Remuchi Oct 23, 2024

Remuchi commented Oct 23, 2024

Mnemotechnician left a comment

Mnemotechnician Oct 25, 2024

Mnemotechnician Oct 25, 2024

Mnemotechnician Oct 25, 2024

Mnemotechnician Oct 25, 2024

	_entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);
	_entityManager.TryGetComponent<NightVisionComponent>(slot.SlotData["eyes"].HeldEntity, out var nightvision);

	_inventory.TryGetSlotEntity(container.Owner, "eyes", out var entityUid) && entityUid == uid)
	_inventory.TryGetSlotEntity(container.Owner, nightvision.Slot, out var entityUid) && entityUid == uid)

NVG (Port From _LostParadise) #1128

Are you sure you want to change the base?

NVG (Port From _LostParadise) #1128

Conversation

BL02DL commented Oct 22, 2024 • edited Loading

Description

TODO

Changelog

github-actions bot commented Oct 22, 2024 • edited Loading

Eagle-0 commented Oct 22, 2024

BL02DL commented Oct 22, 2024

FoxxoTrystan left a comment

Choose a reason for hiding this comment

FoxxoTrystan commented Oct 22, 2024

Remuchi left a comment • edited Loading

Choose a reason for hiding this comment

BL02DL commented Oct 22, 2024

VMSolidus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Remuchi 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

Remuchi commented Oct 23, 2024

Mnemotechnician 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

BL02DL commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024 •

edited

Loading

Remuchi left a comment •

edited

Loading