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.

Sign up for GitHub

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

Jump to bottom

Code maintenace of Stats Entities #4750

Merged
merged 2 commits into from
Nov 7, 2023
Merged

Code maintenace of Stats Entities #4750

merged 2 commits into from
Nov 7, 2023

Conversation

fira
Copy link
Member

@fira fira commented Oct 21, 2023 •
edited by github-actions bot
Loading

About the pull request

  • Removes extraneous = null
  • Adds Destroy with proper clearing to all entities
  • Fixes up some single-letter vars and incorrect proc signatures
  • Transforms some comments to DMDoc
  • Generally tries to clear up the usage of the entity lists (they're MOSTLY indexed)

Explain why it's good for the game

Code maintenance

Testing Photographs and Procedure

Didn't see anything wrong but best tested in TM

Changelog

Nothing player facing

@fira fira added the Code Improvement Make the code longer label Oct 21, 2023
@Drulikar Drulikar added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Oct 21, 2023
Drulikar
Drulikar approved these changes Oct 21, 2023
View reviewed changes
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

Code changes appear fine. Only thing that may come up in testing is if something is nulled that something still tries to access.

@fira
Copy link
Member Author

fira commented Oct 21, 2023

It is mostly a safety net due to GC errors in death statistics being deleted, otherwise entities are very rarely deleted

@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Oct 24, 2023
@cm13-github
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 1, 2023

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Nov 1, 2023
@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Nov 3, 2023
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the Stale beg a maintainer to review your PR label Nov 4, 2023
@stagda stagda mentioned this pull request Nov 6, 2023
Closed
3 tasks
@Cooliecan Cooliecan mentioned this pull request Nov 7, 2023
Closed
3 tasks
@Drulikar Drulikar added this pull request to the merge queue Nov 7, 2023
Merged via the queue into cmss13-devs:master with commit a341a35 Nov 7, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Drulikar Drulikar Drulikar approved these changes

@harryob harryob harryob approved these changes

Assignees
No one assigned
Labels
Code Improvement Make the code longer Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fira @cm13-github @harryob @Drulikar