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

Pacified Zone marker #1815

Merged
merged 17 commits into from
Aug 9, 2024
Merged

Pacified Zone marker #1815

merged 17 commits into from
Aug 9, 2024

Conversation

Vonsant
Copy link
Contributor

@Vonsant Vonsant commented Aug 7, 2024

About the PR

Port this PR Corvax-Frontier#380 made by @BeatusCrow

Why / Balance

Adds a "Pacified" component to all people in a certain radius, except people with prescribed jobs, like security. Can help deal with server raiders who try to massacre an outpost.

How to test

Requires additional review, has not been tested on a live server. Only tested on local.

  1. Spawn marker
  2. Check to see if you have a "Pacified" status.
  3. Step away from the marker.
  4. Check "Pacified" status gone.
  5. Respawn as SecurityGuard or Sheriff
  6. Pacified status is not added when approaching a marker.

Media

https://youtu.be/BdBIEzuLkVk

  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Copy link
Contributor

github-actions bot commented Aug 7, 2024

RSI Diff Bot; head commit 046fc9a merging into c3f7530
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/_NF/Markers/pacifiedzone.rsi

State Old New Status
pacified_zone Added

Edit: diff updated after 046fc9a

@dvir001 dvir001 requested review from Cheackraze and whatston3 August 7, 2024 20:47
@github-actions github-actions bot added the Status: Needs Review This PR is awaiting reviews label Aug 7, 2024
@BeatusCrow
Copy link

BeatusCrow commented Aug 7, 2024

Well, if you need something from me, write.

@whatston3
Copy link
Contributor

It's a nice little feature! I've made a set of suggestions here: whatston3:PacifiedZone-component. I'd accept it with the changes in that link, though it still might have issues with two overlapping zones.

On the head of your feature, at the moment, if anybody starts with the Pacified trait, it'll be removed when they enter and then exit the zone. I've created a new component to track when a zone is exited - might want to track which entity actually spawned it. Also, I don't believe the interval is actually adhered to in the system, and I'm not entirely fond of line 90, had problems with the magnet component using similar logic - if it's created halfway through the shift, it'll run that update every frame until it catches up. I've moved the intermediateListEntries to a local variable and did a bit of cleanup.

Hope the changes seem reasonable, and thanks for the feature. 😃

@BeatusCrow
Copy link

It's a nice little feature! I've made a set of suggestions here: whatston3:PacifiedZone-component. I'd accept it with the changes in that link, though it still might have issues with two overlapping zones.

On the head of your feature, at the moment, if anybody starts with the Pacified trait, it'll be removed when they enter and then exit the zone. I've created a new component to track when a zone is exited - might want to track which entity actually spawned it. Also, I don't believe the interval is actually adhered to in the system, and I'm not entirely fond of line 90, had problems with the magnet component using similar logic - if it's created halfway through the shift, it'll run that update every frame until it catches up. I've moved the intermediateListEntries to a local variable and did a bit of cleanup.

Hope the changes seem reasonable, and thanks for the feature. 😃

You made a good point about removing the pacified feature... I just thought that on the frontier, this component is missing from everyone due to the specifics of the game (it's just illogical to play with pacifism where you need to handle weapons often). I think you can just create a pacifism component tied to the zone itself so that there is no deletion of the main one.
But about the two overlapping zones... Personally, I don't think there will be any problems: if you are within two zones at the same time, then 3 situations are possible:

  1. You are in two zones at once. Then both zones will add you to the list of "creatures in the zone" and give you a component if it was not there before.
  2. You left one zone, but entered another (the first has a radius of 5, and the second has 10). Then one zone will remove the component from you and forget about you, and the second one will add it again.
  3. Well, if you leave two zones, then the component will try to remove the first one that you leave, and then the second one... An error may indeed appear here, but it can be avoided by adding a simple check for the presence of a component.

@BeatusCrow
Copy link

It's a nice little feature! I've made a set of suggestions here: whatston3:PacifiedZone-component. I'd accept it with the changes in that link, though it still might have issues with two overlapping zones.

On the head of your feature, at the moment, if anybody starts with the Pacified trait, it'll be removed when they enter and then exit the zone. I've created a new component to track when a zone is exited - might want to track which entity actually spawned it. Also, I don't believe the interval is actually adhered to in the system, and I'm not entirely fond of line 90, had problems with the magnet component using similar logic - if it's created halfway through the shift, it'll run that update every frame until it catches up. I've moved the intermediateListEntries to a local variable and did a bit of cleanup.

Hope the changes seem reasonable, and thanks for the feature. 😃

I added comments to the code. If you agree with this, then I will quickly supplement the code with my suggestions and yours that you have suggested here whatston3:PacifiedZone-component.

@Vonsant
Copy link
Contributor Author

Vonsant commented Aug 8, 2024

@whatston3 i'm don't know what to do and just waiting your responce for BeatusCrow x)

@github-actions github-actions bot added the FTL label Aug 9, 2024
@whatston3
Copy link
Contributor

whatston3 commented Aug 9, 2024

Made a set of changes to my suggestions and pulled them in:

  • Pacified zones now no longer track a user if they are already pacified, from any source. This results in better behaviour, we no longer need to worry about multiple zones targeting the same user.
    • Minor side effect as of now - a user leaving one zone while overlapped by others will have pacification removed until another zone runs its own update.
  • Added a separate alert (using your nice looking shield sprite). This is shown in the picture below - helps distinguish between regional generation and otherwise.
    • The specific alert to show should probably be defined in the PacifiedZoneComponent, but as of now, it's a constant in the System.
  • Added a ComponentShutdown that removes the pacified status from any tracked entities.

Quite happy with this, looks good in my testing. I'll approve this and give it some time for others to review, though I don't foresee any problems. Thank you again!

image

@BeatusCrow
Copy link

Made a set of changes to my suggestions and pulled them in:

  • Pacified zones now no longer track a user if they are already pacified, from any source. This results in better behaviour, we no longer need to worry about multiple zones targeting the same user.

    • Minor side effect as of now - a user leaving one zone while overlapped by others will have pacification removed until another zone runs its own update.

Thank you for such good feedback :)

@github-actions github-actions bot added Map-Shuttle Map - Shuttle Map-Outpost Map - Outpost and removed YML labels Aug 9, 2024
@dvir001 dvir001 merged commit 1040db4 into new-frontiers-14:master Aug 9, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# FTL Map-Outpost Map - Outpost Map-Shuttle Map - Shuttle Sprites Status: Needs Review This PR is awaiting reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants