View Issue Details

IDProjectCategoryView StatusLast Update
0003031FSSCPuser interfacepublic2014-04-28 11:20
ReporterYarn Assigned ToYarn  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.1 
Summary0003031: [patch] Use PF_USE_AFFIRMATIVE_ICON where necessary
DescriptionIf a pop-up message uses only one button, and that button uses the "affirmative" icon (the one pointing down and typically labeled "Ok" or "Yes"), the PF_USE_AFFIRMATIVE_ICON flag must be set to ensure that the button is placed in the correct position. Most such pop-ups use that flag, but some don't. The attached patch fixes the ones that don't.
Additional InformationI have attached a screenshot of one of these broken pop-ups.
TagsNo tags attached.

Activities

Yarn

2014-04-15 19:55

developer  

PopupFix.patch (12,344 bytes)   
Index: code/controlconfig/controlsconfig.cpp
===================================================================
--- code/controlconfig/controlsconfig.cpp	(revision 10563)
+++ code/controlconfig/controlsconfig.cpp	(working copy)
@@ -1542,7 +1542,7 @@
 				}
 
 				if ((k > 0) && !Config_allowed[k & KEY_MASK]) {
-					popup(0, 1, POPUP_OK, XSTR( "That is a non-bindable key.  Please try again.", 207));
+					popup(PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, XSTR( "That is a non-bindable key.  Please try again.", 207));
 					k = 0;
 				}
 
Index: code/cutscene/cutscenes.cpp
===================================================================
--- code/cutscene/cutscenes.cpp	(revision 10563)
+++ code/cutscene/cutscenes.cpp	(working copy)
@@ -334,7 +334,7 @@
 		else
 			sprintf(str, XSTR("Unable to play movie %s.", 204), Cutscenes[which_cutscene].name);
 
-		popup(0, 1, POPUP_OK, str );
+		popup(PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, str );
 	}
 	
 }
Index: code/freespace2/freespace.cpp
===================================================================
--- code/freespace2/freespace.cpp	(revision 10563)
+++ code/freespace2/freespace.cpp	(working copy)
@@ -8338,7 +8338,7 @@
 			break;
 		} else {
 			// no CD found, so prompt user
-			popup_rval = popup(PF_BODY_BIG, 1, POPUP_OK, XSTR( "FreeSpace 2 CD not found\n\nInsert a FreeSpace 2 CD to continue", 202));
+			popup_rval = popup(PF_BODY_BIG | PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, XSTR( "FreeSpace 2 CD not found\n\nInsert a FreeSpace 2 CD to continue", 202));
 			refresh_files = 1;
 			if ( popup_rval != 1 ) {
 				cd_present = 0;
@@ -8385,7 +8385,7 @@
 			break;
 		} else {
 			// no CD found, so prompt user
-			popup_rval = popup(PF_BODY_BIG, 1, POPUP_OK, XSTR("Please insert CD %d", 1468), cdnum);
+			popup_rval = popup(PF_BODY_BIG | PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, XSTR("Please insert CD %d", 1468), cdnum);
 			refresh_files = 1;
 			if ( popup_rval != 1 ) {
 				cd_present = 0;
Index: code/graphics/gropenglextension.cpp
===================================================================
--- code/graphics/gropenglextension.cpp	(revision 10563)
+++ code/graphics/gropenglextension.cpp	(working copy)
@@ -420,7 +420,7 @@
 			Use_GLSL = 0;
 			mprintf(("  OpenGL Shading Language version %s is not sufficient to use GLSL mode in FSO. Defaulting to fixed-function renderer.\n", glGetString(GL_SHADING_LANGUAGE_VERSION_ARB) ));
 #ifdef NDEBUG
-			popup(0, 1, POPUP_OK, "GLSL support not available on this GPU. Disabling shader support and defaulting to fixed-function rendering.\n");
+			popup(PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, "GLSL support not available on this GPU. Disabling shader support and defaulting to fixed-function rendering.\n");
 #endif
 		}
 	}
Index: code/menuui/mainhallmenu.cpp
===================================================================
--- code/menuui/mainhallmenu.cpp	(revision 10563)
+++ code/menuui/mainhallmenu.cpp	(working copy)
@@ -282,24 +282,24 @@
 	error = psnet_get_network_status();
 	switch( error ) {
 	case NETWORK_ERROR_NO_TYPE:
-		popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have not defined your type of Internet connection.  Please run the Launcher, hit the setup button, and go to the Network tab and choose your connection type.", 360));
+		popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have not defined your type of Internet connection.  Please run the Launcher, hit the setup button, and go to the Network tab and choose your connection type.", 360));
 		break;
 	case NETWORK_ERROR_NO_WINSOCK:
-		popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "Winsock is not installed.  You must have TCP/IP and Winsock installed to play multiplayer FreeSpace.", 361));
+		popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "Winsock is not installed.  You must have TCP/IP and Winsock installed to play multiplayer FreeSpace.", 361));
 		break;
 	case NETWORK_ERROR_NO_PROTOCOL:
 		if (Multi_options_g.protocol == NET_TCP) {
-			popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "TCP/IP protocol not found.  This protocol is required for multiplayer FreeSpace.", 1602));
+			popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "TCP/IP protocol not found.  This protocol is required for multiplayer FreeSpace.", 1602));
 		} else {
 			Assert(Multi_options_g.protocol == NET_IPX);
-			popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "IPX protocol not found.  This protocol is required for multiplayer FreeSpace.", 1603));
+			popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "IPX protocol not found.  This protocol is required for multiplayer FreeSpace.", 1603));
 		}
 		break;
 	case NETWORK_ERROR_CONNECT_TO_ISP:
-		popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected Dial Up Networking as your type of connection to the Internet.  You are not currently connected.  You must connect to your ISP before continuing on past this point.", 363));
+		popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected Dial Up Networking as your type of connection to the Internet.  You are not currently connected.  You must connect to your ISP before continuing on past this point.", 363));
 		break;
 	case NETWORK_ERROR_LAN_AND_RAS:
-		popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have indicated that you use a LAN for networking.  You also appear to be dialed into your ISP.  Please disconnect from your service provider, or choose Dial Up Networking.", 364));
+		popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have indicated that you use a LAN for networking.  You also appear to be dialed into your ISP.  Please disconnect from your service provider, or choose Dial Up Networking.", 364));
 		break;
 
 	case NETWORK_ERROR_NONE:
@@ -310,15 +310,15 @@
 	// if our selected protocol is not active
 	if ((Multi_options_g.protocol == NET_TCP) && !Tcp_active) {
 		if (Tcp_failure_code == WSAEADDRINUSE) {
-			popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected TCP/IP for multiplayer FreeSpace, but the TCP socket is already in use.  Check for another instance and/or use the \"-port <port_num>\" command line option to select an available port.", 1604));
+			popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected TCP/IP for multiplayer FreeSpace, but the TCP socket is already in use.  Check for another instance and/or use the \"-port <port_num>\" command line option to select an available port.", 1604));
 		} else {
-			popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected TCP/IP for multiplayer FreeSpace, but the TCP/IP protocol was not detected on your machine.", 362));
+			popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected TCP/IP for multiplayer FreeSpace, but the TCP/IP protocol was not detected on your machine.", 362));
 		}
 		return;
 	}
 
 	if ((Multi_options_g.protocol == NET_IPX) && !Ipx_active) {
-		popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected IPX for multiplayer FreeSpace, but the IPX protocol was not detected on your machine.", 1402));
+		popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You have selected IPX for multiplayer FreeSpace, but the IPX protocol was not detected on your machine.", 1402));
 		return;
 	}
 
@@ -329,7 +329,7 @@
 	// 7/9/98 -- MWA.  Deal with the connection speed issue.  make a call to the multiplayer code to
 	// determine is a valid connection setting exists
 	if (Multi_connection_speed == CONNECTION_SPEED_NONE) {
-		popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You must define your connection speed.  Please run the Launcher, hit the setup button, and go to the Network tab and choose your connection speed.", 986) );
+		popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "You must define your connection speed.  Please run the Launcher, hit the setup button, and go to the Network tab and choose your connection speed.", 986) );
 		return;
 	}
 
Index: code/menuui/playermenu.cpp
===================================================================
--- code/menuui/playermenu.cpp	(revision 10563)
+++ code/menuui/playermenu.cpp	(working copy)
@@ -329,7 +329,7 @@
 	if ((Global_warning_count > 10 || Global_error_count > 0) && !Startup_warning_dialog_displayed) {
 		char text[512];
 		sprintf(text, "Warning!\n\nThe currently active mod has generated %d warnings and/or errors during program startup.  These could have been caused by anything from incorrectly formated table files to corrupt models.  While FreeSpace Open will attempt to compensate for these issues, it cannot guarantee a trouble-free gameplay experience.  Source Code Project staff cannot provide assistance or support for these problems, as they are caused by the mod's data files, not FreeSpace Open's source code.", Global_warning_count + Global_error_count);
-		popup(PF_TITLE_BIG | PF_TITLE_RED, 1, POPUP_OK, text);
+		popup(PF_TITLE_BIG | PF_TITLE_RED | PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, text);
 		Startup_warning_dialog_displayed = true;
 	}
 		
Index: code/menuui/readyroom.cpp
===================================================================
--- code/menuui/readyroom.cpp	(revision 10563)
+++ code/menuui/readyroom.cpp	(working copy)
@@ -873,13 +873,13 @@
 	if (mc_rval == -1)
 	{  // is campaign and next mission valid?
 		gamesnd_play_iface(SND_GENERAL_FAIL);
-		popup(0, 1, POPUP_OK, XSTR( "The campaign is over.  To replay the campaign, either create a new pilot or restart the campaign in the campaign room.", 112) );
+		popup(PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, XSTR( "The campaign is over.  To replay the campaign, either create a new pilot or restart the campaign in the campaign room.", 112) );
 		return -1;
 	}
 	else if(mc_rval == -2)
 	{
 		gamesnd_play_iface(SND_GENERAL_FAIL);
-		popup(0, 1, POPUP_OK, NOX("The current campaign has no missions") );
+		popup(PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, NOX("The current campaign has no missions") );
 		return -1;
 	}
 
@@ -928,7 +928,7 @@
 
 		case CAMPAIGN_TAB:
 			if ( !strlen(Campaign.filename) ) {
-				popup( PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "The currently active campaign cannot be found, unable to switch to campaign mode!", 1612));
+				popup( PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, XSTR( "The currently active campaign cannot be found, unable to switch to campaign mode!", 1612));
 				break;
 			}
 
Index: code/mission/missionparse.cpp
===================================================================
--- code/mission/missionparse.cpp	(revision 10563)
+++ code/mission/missionparse.cpp	(working copy)
@@ -5553,7 +5553,7 @@
 	if ((saved_warning_count - Global_warning_count) > 10 || (saved_error_count - Global_error_count) > 0) {
 		char text[512];
 		sprintf(text, "Warning!\n\nThe current mission has generated %d warnings and/or errors during load.  These are usually caused by corrupted ship models or syntax errors in the mission file.  While FreeSpace Open will attempt to compensate for these issues, it cannot guarantee a trouble-free gameplay experience.  Source Code Project staff cannot provide assistance or support for these problems, as they are caused by the mission's data files, not FreeSpace Open's source code.", (saved_warning_count - Global_warning_count) + (saved_error_count - Global_error_count));
-		popup(PF_TITLE_BIG | PF_TITLE_RED | PF_NO_NETWORKING, 1, POPUP_OK, text);
+		popup(PF_TITLE_BIG | PF_TITLE_RED | PF_USE_AFFIRMATIVE_ICON | PF_NO_NETWORKING, 1, POPUP_OK, text);
 	}
 
 	log_printf(LOGFILE_EVENT_LOG, "Mission %s loaded.\n", pm->name); 
Index: code/network/multiui.cpp
===================================================================
--- code/network/multiui.cpp	(revision 10563)
+++ code/network/multiui.cpp	(working copy)
@@ -1878,7 +1878,7 @@
 
 	// 5/26/98 -- for team v team games, don't allow ingame joining :-(
 	if ( (Multi_join_selected_item->flags & AG_FLAG_TEAMS) && (Multi_join_selected_item->flags & (AG_FLAG_PAUSE|AG_FLAG_IN_MISSION)) ) {
-		popup(0, 1, POPUP_OK, XSTR("Joining ingame is currently not allowed for team vs. team games",772));
+		popup(PF_USE_AFFIRMATIVE_ICON, 1, POPUP_OK, XSTR("Joining ingame is currently not allowed for team vs. team games",772));
 		return;
 	}
 
PopupFix.patch (12,344 bytes)   

Yarn

2014-04-15 19:56

developer  

PopupButton.png (24,678 bytes)   
PopupButton.png (24,678 bytes)   

niffiwan

2014-04-28 11:20

developer   ~0015715

Last edited: 2014-04-28 11:20

Looks good to me. I checked several of the fixed popups and confirmed that the OK button was placed correctly after applying the patch. I also checked that all calls to popup() with a single button now used PF_USE_AFFIRMATIVE_ICON, and they all do.

Will commit shortly...

niffiwan

2014-04-28 11:20

developer   ~0015716

Fix committed to trunk@10601.

Related Changesets

fs2open: trunk r10601

2014-04-28 06:31

niffiwan


Ported: N/A

Details Diff
From Yarn: Fix mantis 3031

Use PF_USE_AFFIRMATIVE_ICON where necessary
Affected Issues
0003031
mod - /trunk/fs2_open/code/controlconfig/controlsconfig.cpp Diff File
mod - /trunk/fs2_open/code/cutscene/cutscenes.cpp Diff File
mod - /trunk/fs2_open/code/freespace2/freespace.cpp Diff File
mod - /trunk/fs2_open/code/graphics/gropenglextension.cpp Diff File
mod - /trunk/fs2_open/code/menuui/mainhallmenu.cpp Diff File
mod - /trunk/fs2_open/code/menuui/playermenu.cpp Diff File
mod - /trunk/fs2_open/code/menuui/readyroom.cpp Diff File
mod - /trunk/fs2_open/code/mission/missionparse.cpp Diff File
mod - /trunk/fs2_open/code/network/multiui.cpp Diff File

Issue History

Date Modified Username Field Change
2014-04-15 19:55 Yarn New Issue
2014-04-15 19:55 Yarn File Added: PopupFix.patch
2014-04-15 19:56 Yarn File Added: PopupButton.png
2014-04-20 09:30 Echelon9 Status new => code review
2014-04-28 11:18 niffiwan Assigned To => Yarn
2014-04-28 11:18 niffiwan Status code review => assigned
2014-04-28 11:20 niffiwan Note Added: 0015715
2014-04-28 11:20 niffiwan Note Edited: 0015715
2014-04-28 11:20 niffiwan Changeset attached => fs2open trunk r10601
2014-04-28 11:20 niffiwan Note Added: 0015716
2014-04-28 11:20 niffiwan Status assigned => resolved
2014-04-28 11:20 niffiwan Resolution open => fixed