Skip to content

Conversation

@Goober5000
Copy link
Contributor

There are three popups of decreasing priority that can appear in the main hall screen: campaign not found, pilot upgraded, and player tip. Only one of these should actually be displayed -- the first popup will take the player to a different screen, rendering subsequent popups useless; and the second popup is more important than the third popup. This PR prioritizes the popups accordingly.

Yes, this uses goto. It's a succinct and clear way of implementing the necessary logic. It works better than both a one-iteration loop and a cascading if() block.

Fixes #4418.

@Goober5000 Goober5000 added cleanup A modification or rewrite of code to make it more understandable or easier to maintain. interface labels Jun 17, 2022
Copy link
Member

@jg18 jg18 left a comment

Choose a reason for hiding this comment

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

Instead of goto, how about adding an extra bool flag around line 1061 like

bool popup_shown = false;

then set popup_shown to true on line 1084, and then change lines 1088-1094 to something like this (comments omitted for brevity):

	if (!popup_shown) {
		popup_shown = player_tips_controls();
	}

	if (!popup_shown) {
		player_tips_popup();
	}

Also, to fully appreciate the danger of goto, see https://xkcd.com/292/

@Goober5000
Copy link
Contributor Author

Goober5000 commented Jun 17, 2022

Yeah, that'll work. It's a sequential if() block, which works better here than a cascading if() block.

My gotos are immune from dinosaur attacks. :P

There are three popups of decreasing priority that can appear in the main hall screen: campaign not found, pilot upgraded, and player tip.  Only one of these should actually be displayed -- the first popup will take the player to a different screen, rendering subsequent popups useless; and the second popup is more important than the third popup.  This PR prioritizes the popups accordingly.
@Goober5000 Goober5000 force-pushed the main_hall_popup_priority branch from 9bdd01f to b8231a4 Compare June 17, 2022 17:25
Copy link
Member

@jg18 jg18 left a comment

Choose a reason for hiding this comment

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

Thanks, looks good now.

@Goober5000 Goober5000 merged commit b326980 into scp-fs2open:master Jun 18, 2022
@Goober5000 Goober5000 deleted the main_hall_popup_priority branch June 18, 2022 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup A modification or rewrite of code to make it more understandable or easier to maintain.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Player tips displayed after "campaign not found" popup

2 participants