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

Fix weapon drop callback breaking SP #797

Merged
merged 3 commits into from
Mar 26, 2024

Conversation

itscynxx
Copy link
Contributor

@itscynxx itscynxx commented Mar 5, 2024

The functions for both weapon drop callbacks are in an #if MP statement, but the globalization wasn't, causing SP to try to add the functions, which it didn't find as MP wasn't active. Without this change, you compile error before SP finishes loading

Sidenote: genuinely wtf is that last line diff. I didn't touch it??
sideline 2: ok i did actually touch it now but it still shows the exact same thing even after removing the newline, idk man

Related:

Copy link
Contributor

@uniboi uniboi left a comment

Choose a reason for hiding this comment

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

Is there a reason the callbacks aren't defined for both SP and MP?

@GeckoEidechse
Copy link
Member

Is there a reason the callbacks aren't defined for both SP and MP?

Probably not, cc @NoCatt to be sure ig
c.f.: #692

@itscynxx
Copy link
Contributor Author

itscynxx commented Mar 5, 2024

Is there a reason the callbacks aren't defined for both SP and MP?

I can't be 100% sure as it's not my code, but looking at the original pr it's used in MP to destroy weapon drops if they aren't allowed, instead of the solution which apparently didn't always work before

More than willing to edit this to work for SP also, but I was just concerned on making the game not crash

@GeckoEidechse
Copy link
Member

bump

@itscynxx
Copy link
Contributor Author

bump

I haven't made any changes due to not having a reply on editing the code to instead work on SP

I suppose the main question would be do we want to move the functions to be above the #if MP they're currently inside of, or to end the #if MP before the functions (as presumably the functions above are vanilla/replicating vanilla? so we would keep modded functions separate)

@Zanieon
Copy link
Contributor

Zanieon commented Mar 26, 2024

Moving those functions inside the #if MP is alright because campaign has its own call of those functions in _base_gametype_sp.gnut script which is loaded only in SP.

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.

I'm just gonna assume this was tested to some degree by @itscynxx
At least I have no way of testing myself rn and this PR has been stuck for way too long...

Trusting @Zanieon on their comment and approval.

@GeckoEidechse GeckoEidechse merged commit a8c0653 into R2Northstar:main Mar 26, 2024
3 checks passed
@Bobbyperson
Copy link
Contributor

can confirm that this works completely fine on singleplayer, played through most of a mission, executed a grunt, no issues at all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants