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 Infinite Loading Screen + Fix Modsettings "int" functionality #651

Closed
wants to merge 29 commits into from

Conversation

EnderBoy9217
Copy link
Contributor

@EnderBoy9217 EnderBoy9217 commented Jun 17, 2023

Previously if a server crashed while the client was in the map loading screen, all clients would be stuck in an infinite loading screen. (This was reproduced by multiple people)

This boots clients back to the menu if the game hasn't loaded within 30 seconds (server crash) and loads the main menu, it doesn't affect the campaign's "continue" feature either.

Video below (some footage sped up by 8x to reduce video size)

Loading.Screen.Fix.mp4

EDIT:
This adds a Northstar.Client section to modsettings
This also fixes an issue where modsettings cannot take an "int" ConVar
Closes: R2Northstar/Northstar#489

@uniboi
Copy link
Contributor

uniboi commented Jun 17, 2023

Maybe open a dialog when returning to the main menu that tells the user that the loading has timed out

@EnderBoy9217
Copy link
Contributor Author

Added an error message for when this happens
Desktop Screenshot 2023 06 17 - 11 37 35 46

@F1F7Y
Copy link
Member

F1F7Y commented Jun 17, 2023

My main concern would be what if the loading actually takes more than 30 seconds?

Please also comment your code, scripts are already chaotic the way they are right now

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.

I think 30 seconds before a timeout is fine

Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
@EnderBoy9217
Copy link
Contributor Author

EnderBoy9217 commented Jun 17, 2023

Added the comments and uni's suggestions, also added a loop to reduce the chance of a false positive from loading multiple times
Unless anyone knows a way to cancel the thread with another function instantly

@uniboi
Copy link
Contributor

uniboi commented Jun 17, 2023

is there a point in aborting the function early? I'd just keep the wait threshold because letting the function run is cheaper than checking every second if the client is still loading.
Maybe there's a signal we can use, that'd be more elegant

@ASpoonPlaysGames
Copy link
Contributor

Maybe there's a signal we can use, that'd be more elegant

there are various signals that might be of use (all of which are called on uiGlobal.signalDummy)

LevelShutdown
LevelFinishedLoading

i think LevelShutdown is best for this tbh

@uniboi
Copy link
Contributor

uniboi commented Jun 17, 2023

LevelShutdown triggers right before the map unloads iirc.
I'd make the function something like this

void function TimeoutHandler()
{
	const int timeoutThreshold = 30 // how long this function will wait before disconnecting
	EndSignal( uiGlobal.signalDummy, "LevelFinishedLoading" ) // cancel the thread if the level has finished loading

	wait timeoutThreshold;

	if ( uiGlobal.isLoading == true ) // Disconnect if the client still hasn't connected
	{
		printt( "TimeoutHandler disconnected because loading took too long" )
		UICodeCallback_LevelLoadingFinished( true ) // Removes loading screen
		Disconnect() // Loads main menu
		OpenErrorDialog( format( "Loading timed out (Over %i seconds)", timeoutThreshold ) ) // Opens error dialog to notify user
	}
}

@EnderBoy9217
Copy link
Contributor Author

EnderBoy9217 commented Jun 17, 2023

couldnt we get rid of the "if" statement if that signal ends the script
As that signal is sent right after isLoading becomes false

Edit: (nevermind 1 other script can change the bool)

@EnderBoy9217
Copy link
Contributor Author

EnderBoy9217 commented Jun 17, 2023

endsignal causes the script to not work @uniboi , it will print that it is attempting to send the player back to lobby "TimeoutHandler disconnected because loading took too long" but it won't

Is there any other known way to end the script, or should we go back to the for loop

@uniboi
Copy link
Contributor

uniboi commented Jun 17, 2023

Try changing the function to this:

void function TimeoutHandler()
{
	const int timeoutThreshold = 30 // how long this function will wait before disconnecting
	EndSignal( uiGlobal.signalDummy, "LevelFinishedLoading" ) // cancel the thread if the level has finished loading

	OnThreadEnd(
		void function()
		{
			if ( uiGlobal.isLoading == true ) // Disconnect if the client still hasn't connected
			{
				printt( "TimeoutHandler disconnected because loading took too long" )
				UICodeCallback_LevelLoadingFinished( true ) // Removes loading screen
				Disconnect() // Loads main menu
				OpenErrorDialog( format( "Loading timed out (Over %i seconds)", timeoutThreshold ) ) // Opens error dialog to notify user
			}
		}
	)

	wait timeoutThreshold;
}

@EnderBoy9217
Copy link
Contributor Author

EnderBoy9217 commented Jun 17, 2023

I have no idea what you did there @uniboi
But it didn't load the main menu ui, only the dialog and .bik file

@EnderBoy9217
Copy link
Contributor Author

The current code still works but likely isn't the most optimal, if anyone has an alternative that doesn't use EndSignal (which seems to be causing issues) let me know

@uniboi
Copy link
Contributor

uniboi commented Jun 18, 2023

I'll take a look tomorrow

@GeckoEidechse
Copy link
Member

Added the comments and uni's suggestions, also added a loop to reduce the chance of a false positive from loading multiple times.

Make sure to click the Resolve button on the conversations where it's applicable so we know that part has been addressed ^^

@F1F7Y
Copy link
Member

F1F7Y commented Jul 9, 2023

I'll take a look tomorrow

@uniboi bump

@F1F7Y F1F7Y added needs code review Changes from PR still need to be reviewed in code needs testing Changes from the PR still need to be tested labels Jul 9, 2023
@EnderBoy9217
Copy link
Contributor Author

EnderBoy9217 commented Aug 25, 2023

Added ConVar and per Spoon's request added it to modsettings

This also adds a Northstar.Client section to modsettings

This PR now fixes an issue where modsettings cannot take an "int" ConVar

Put both of these in PR description as well

@EnderBoy9217 EnderBoy9217 changed the title Fix Infinite Loading Screen Fix Infinite Loading Screen + Fix Modsettings "int" functionality Aug 26, 2023
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
@EnderBoy9217
Copy link
Contributor Author

Changed it from an int to a float

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.

Just a few more things from me

Northstar.Client/mod.json Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
Northstar.Client/mod/scripts/vscripts/ui/_menus.nut Outdated Show resolved Hide resolved
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 :)

@ASpoonPlaysGames ASpoonPlaysGames removed the needs code review Changes from PR still need to be reviewed in code label Aug 26, 2023
@EnderBoy9217
Copy link
Contributor Author

I got an odd error while playing with this and I think its related
image
nslog2023-08-27 12-09-30.txt

@ASpoonPlaysGames
Copy link
Contributor

image
indeed it is

@EnderBoy9217
Copy link
Contributor Author

I believe its fixed with an "IsConnected()" check I just added, works fine in testing

@EnderBoy9217
Copy link
Contributor Author

Also this fix works if the masterserver was to go down while someone was loading for some reason

Copy link
Member

@F1F7Y F1F7Y left a comment

Choose a reason for hiding this comment

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

Blocking as:

  1. Each fix should be a separate PR
  2. The "fix" for infinite loading screen is really hacky and should be handled in native by checking the connection like valve source does.

@ASpoonPlaysGames ASpoonPlaysGames added the waiting on changes by author Waiting on PR author to implement the suggested changes label Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs testing Changes from the PR still need to be tested waiting on changes by author Waiting on PR author to implement the suggested changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Infinite Loading on Server Crash
5 participants