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

Complete overhaul for the CTF script #830

Merged
merged 18 commits into from
Sep 2, 2024

Conversation

Zanieon
Copy link
Contributor

@Zanieon Zanieon commented Aug 2, 2024

This PR is required for #829 to compile check properly, so imma already do it as well.

Yes yes @GeckoEidechse i know you hate me dumping all changes in one go, but i actually reordered some funcions and nested them with the ASCII art titles as i usually do, that means the diff will always be huge cuz github dumb to know code was reordered.

@Zanieon Zanieon added gamemode Issues related to gamemodes, custom or vanilla needs testing Changes from the PR still need to be tested labels Aug 2, 2024
@GeckoEidechse
Copy link
Member

Looked through this here and there over the past few days with the intention of splicing out individual changes and so far was rather unsuccessful in particular cause of how many lines this touches.

I'm really hesitant to merge any changes I don't fully understand so I'm trying to figure out the best way to merge this atm.
(I really want a Squirrel auto-formatter already so that I can at least weed out the formatting changes...)

Maybe we should go over the changes together in vc or something?

@Zanieon
Copy link
Contributor Author

Zanieon commented Aug 8, 2024

Maybe we should go over the changes together in vc or something?

I'm down for it so i can tell what the changes does yeah.

@github-actions github-actions bot added the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Aug 30, 2024
GeckoEidechse pushed a commit that referenced this pull request Aug 30, 2024
in an effort to have all related functions close to each other

Part of the refactor in #830
GeckoEidechse pushed a commit that referenced this pull request Aug 30, 2024
as part of the refactoring effort in #830
GeckoEidechse pushed a commit that referenced this pull request Aug 30, 2024
as part of the refactoring effort in #830
GeckoEidechse pushed a commit that referenced this pull request Aug 30, 2024
as part of the refactoring effort in #830
GeckoEidechse pushed a commit that referenced this pull request Aug 30, 2024
as part of the refactoring effort in #830
@github-actions github-actions bot removed the merge conflicts Blocked by merge conflicts, waiting on the author to resolve label Aug 30, 2024
Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

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

Some nitpicks and some questions, logic overall looks good though

Copy link
Contributor

@ASpoonPlaysGames ASpoonPlaysGames left a comment

Choose a reason for hiding this comment

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

Code looks good!

@GeckoEidechse GeckoEidechse added needs code review Changes from PR still need to be reviewed in code and removed needs testing Changes from the PR still need to be tested labels Aug 30, 2024
Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Confirmed working in testing when playing a few matches with @ASpoonPlaysGames @RoyalBlue1 @Alystrasz @eve-v0

@GeckoEidechse GeckoEidechse self-assigned this Sep 2, 2024
@GeckoEidechse
Copy link
Member

Aight, we spliced out as much as possible. For the last remaining changes, we decided in VC to merge it altogether and fix any remaining bugs post merge.
It was playtested in its current state so hopefully there's no major issues ^^

@GeckoEidechse GeckoEidechse merged commit 957f6a3 into R2Northstar:main Sep 2, 2024
3 checks passed
@Zanieon Zanieon deleted the ctf_overhaul branch September 3, 2024 00:02
@GeckoEidechse GeckoEidechse removed the needs code review Changes from PR still need to be reviewed in code label Sep 24, 2024
@GeckoEidechse GeckoEidechse removed their assignment Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gamemode Issues related to gamemodes, custom or vanilla
Projects
Archived in project
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants