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

Make things pulled by players not despawn when vgroids/uivs/derelicts despawn #2702

Merged
merged 9 commits into from
Jan 23, 2025

Conversation

Alkheemist
Copy link
Contributor

About the PR

Added things that are currently being pulled to the list of things to not delete when a grid is dissipated.

Why / Balance

If player doesn't get bluespaced, why should the things they're holding onto get bluespaced?

How to test

Stand on a grid while pulling something
Wait for the grid to despawn
See that thing you are dragging persist, despite the horrors

Media

dotnet_14_23-38-58.mp4

Requirements

Breaking changes

None identified.

Changelog

🆑

  • tweak: Objects being pulled by someone when a VGroid/UIV/Derelict disappears should stick around.

@github-actions github-actions bot added the C# label Jan 14, 2025
@dvir001
Copy link
Contributor

dvir001 commented Jan 14, 2025

LinkedLifecycleGridSystem.cs is already frontier only, so no need to add frontier comments in it.

@dvir001 dvir001 requested a review from whatston3 January 14, 2025 13:00
@github-actions github-actions bot added the S: Needs Review This PR is awaiting reviews label Jan 14, 2025
@whatston3
Copy link
Contributor

Suggested refactor up here: https://github.com/Alkheemist/frontier-station-14/compare/saving-dragged...whatston3:frontier-station-14:2025-01-21-drag-checks?expand=1

The check for pulled objects is moved to its own function and called from the player character, borg, and other mind container pool. The handled set is renamed for its purpose of skipping already examined mind containers.

Since people can pull people (can pull people...), the duplicates are accounted for at the time of the unparenting - we double check that the entity isn't already in/out of nullspace.

Easy test:
Spawn the vault, summon a bunch of urists/borgs/pun puns. (you'll need clients if you want the borgs/pun puns to stick around). Start a conga line. End the game rule, the entire conga line should stay.

@Alkheemist
Copy link
Contributor Author

Refactor looks good at first glance, I'll take a deeper look into it now. Sorry about the frontier comments, for some reason I thought this was upstream code and wanted to change it minimally to prevent merge issues but since this is wholly frontier station code a refactor is good. (yes I know it's in the _NF folder, I just didn't notice that until now)

Copy link
Contributor Author

@Alkheemist Alkheemist left a comment

Choose a reason for hiding this comment

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

OK, took a look at the refactor, everything seems to be good to me.
We could probably avoid duplicate list entries by using another hashset instead but on the scale of how often this occurs and how few entities being preserved a typical grid being deleted has (maybe 2-3 on a derelict, 5-8 on a vgroid) the performance difference is negligible.

@Alkheemist Alkheemist requested a review from whatston3 January 21, 2025 12:34
@whatston3 whatston3 added S: Approved and removed S: Needs Review This PR is awaiting reviews labels Jan 21, 2025
@whatston3
Copy link
Contributor

whatston3 commented Jan 21, 2025

Thought about this a little - there should be fewer minds than there are mindcontainers, could get the body of anything with a mind and not vice versa.

@dvir001 dvir001 self-requested a review January 23, 2025 23:52
@github-actions github-actions bot added the S: Needs Review This PR is awaiting reviews label Jan 23, 2025
@whatston3 whatston3 merged commit 86723ae into new-frontiers-14:master Jan 23, 2025
13 checks passed
FrontierATC added a commit that referenced this pull request Jan 23, 2025
@Alkheemist
Copy link
Contributor Author

lmao rip, I just implemented checking for minds instead of mindcontainers. I guess that can be a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# S: Approved S: Needs Review This PR is awaiting reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants