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

Fix dropship related crash #662

Merged
merged 14 commits into from
Jul 10, 2023
Merged

Fix dropship related crash #662

merged 14 commits into from
Jul 10, 2023

Conversation

EnderBoy9217
Copy link
Contributor

@EnderBoy9217 EnderBoy9217 commented Jul 7, 2023 •
edited
Loading

Adds a failsafe to fix the bug identified in #588.

ikhadhonger and CTalvio have already been running this code as a mod for over a month

Closes #588

@GeckoEidechse GeckoEidechse changed the title #588 Patch Fix dropship related crash Jul 7, 2023
@EnderBoy9217
Copy link
Contributor Author

Server with has run with this for 13 hours without error, previously it had crashed 4 times within 12 hours.

@ASpoonPlaysGames
Copy link
Contributor

How? Aren't asserts disabled in our build?

@EnderBoy9217
Copy link
Contributor Author

Should I rewrite it just because we don't have a proper way to test?

@ASpoonPlaysGames
Copy link
Contributor

Yeah, but dont do it in cplayer, the original issue is coming from SpawnPlayerIntoDropship() in mp/_classic_mp_dropship_intro.gnut line 191
Add a check there, not here

@EnderBoy9217
Copy link
Contributor Author

EnderBoy9217 commented Jul 8, 2023 •
edited
Loading

Changed the fix to what @ASpoonPlaysGames suggested
This turned out to be the same fix ikhadhonger also used to fix their servers
Although given the nature of this bug it is hard to test, ikhadhonger has been running this code for quite some time on all of their servers

@EnderBoy9217
Copy link
Contributor Author

one thing I will note is these couple of lines before

if ( IsAlive( player ) )
		player.Die() // kill them so we don't have any issues respawning them later

@F1F7Y F1F7Y added the waiting on changes by author Waiting on PR author to implement the suggested changes label Jul 9, 2023
F1F7Y
F1F7Y approved these changes Jul 9, 2023
View reviewed changes
Copy link
Member

@F1F7Y F1F7Y left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Pinged mentioned parties in discord to verify this, code looks good

@F1F7Y F1F7Y merged commit 41f104e into R2Northstar:main Jul 10, 2023
3 checks passed
@F1F7Y
Copy link
Member

F1F7Y commented Jul 10, 2023

Both server hosters verified this fix works

GeckoEidechse pushed a commit that referenced this pull request Jul 10, 2023
@EnderBoy9217 @GeckoEidechse
Fix dropship related crash (#662)
2d073a3 
* Fix infinite loading screen

* Update Northstar.Client/mod/scripts/vscripts/ui/_menus.nut

* Update _menus.nut

* Update _menus.nut

* Update _menus.nut

* Add files via upload

* Update _menus.nut

* Update _menus.nut

* Update cplayer.nut

* Update cplayer.nut

* Add files via upload

* Update _classic_mp_dropship_intro.gnut
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ASpoonPlaysGames ASpoonPlaysGames ASpoonPlaysGames left review comments

@F1F7Y F1F7Y F1F7Y approved these changes

Assignees
No one assigned
Labels
waiting on changes by author Waiting on PR author to implement the suggested changes
Projects
Northstar Roadmap
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dropship related crash
3 participants
@EnderBoy9217 @ASpoonPlaysGames @F1F7Y