View Issue Details

IDProjectCategoryView StatusLast Update
0003048FSSCPuser interfacepublic2014-06-09 01:22
ReporterScotty Assigned ToMageKing17  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.2 RC1 
Summary0003048: Perpetual "More" indication in debriefings
DescriptionExactly what is says on the tin. During a debriefing that extends beyond the bottom of the window (whether alone or with recommendations), the "More" indication displays as normal. However, reaching the bottom of the debriefing is effectively impossible. I was able to click continuously downward without reaching any sort of end (the "More" button as still displayed), and required an equal amount of upward clicks in order to return.
Steps To ReproducePlay Tenebra, encounter debriefing.
Additional InformationHave not tested with retail campaigns or campaigns other than Tenebra. Issue is persistent in Tenebra through several debriefings.
TagsNo tags attached.

Activities

Lorric

2014-06-03 18:41

reporter   ~0015788

I can tell you this is not Tenebra-exclusive, I saw this when I was working on Frontlines.

Goober5000

2014-06-04 03:16

administrator   ~0015789

I've seen this in other places as well. This is just an improper calculation of the number of lines needed by the briefing interface.

MageKing17

2014-06-04 05:22

developer   ~0015790

The "more" is supposed to be permanently visible, based on the behavior of the briefing/command briefing screens, so I've attached two patches. The first one just fixes the infinite-scrolling bug to bring the debriefing screen in line with the other briefing scrolling behaviors.

The second patch gives a visual clue that you've hit the bottom by rendering the "more" as dim red. I've tested both patches separately.

MageKing17

2014-06-04 22:10

developer   ~0015792

After some more thought, I've made a different version of the second patch that centralizes the colors via "#define"s in globalincs/alphacolors.h so that changing from, say, red to bright red won't require changing the code in three places. Really, though, all of those colors should probably be user-definable instead of hardcoded "#define"s... a new section of colors.tbl might be in order.

niffiwan

2014-06-05 00:02

developer   ~0015793

I've tested missiondebrief.cpp.patch and dim_more.patch. Both worked as expected. I can test dim_more2.patch tonight; then commit them.

As for making the colours customisable via colors.tbl, I think that would be best dealt with as a separate feature.

Goober5000

2014-06-05 00:51

administrator   ~0015795

>> The "more" is supposed to be permanently visible

No, this is incorrect. In retail, "more" will disappear when you get to the bottom of the text, although it will still scroll two more lines. The ideal fix would be to disallow the scrolling for two more lines so that "more" disappears exactly when the text runs out which is also exactly when scrolling stops.

In FS1, the scrolling two more lines problem is not present -- scrolling stops when the text stops. However, FS1 does not use a "more" indicator.

MageKing17

2014-06-05 01:54

developer   ~0015796

Based on conversation with Goober5000 (and a quick glance at the retail code), I've uploaded yet another patch. This makes "more" disappear when you reach the bottom, not just turn dim.

It also changes all three briefing screens to use the same offset-by-2 so that you can't scroll down an extra two lines. The question is why, exactly, the offset-by-2 is neccessary in the first place.

niffiwan

2014-06-05 11:11

developer   ~0015802

From some tests I've just run, I think we'll need some more investigation into that offset-by-2 thing. I used a mission with the following:

cmdbrief: 58 lines - chops off the last 3 lines
briefing: 58 lines - chops off the last line
debrief: 58 lines - adds one extra blank line

In all cases the scrolling stops & more vanishes at the same time.

MageKing17

2014-06-06 03:24

developer   ~0015805

Given a more rigorous testing mission, I found that each briefing screen seems to need its own offset. New version of the patch uploaded.

MageKing17

2014-06-06 04:45

developer  

remove_more.patch (5,311 bytes)   
Index: code/globalincs/alphacolors.h
===================================================================
--- code/globalincs/alphacolors.h	(revision 10757)
+++ code/globalincs/alphacolors.h	(working copy)
@@ -38,6 +38,8 @@
 #define Color_text_active_hi	Color_bright_white
 // text drawn as a heading for a section or title, etc
 #define Color_text_heading		Color_violet_gray
+// color of the "more" indicator in briefings/command briefings/debrefings
+#define Color_more_indicator	Color_red
 
 #define Color_bright Color_bright_blue
 #define Color_normal Color_white
Index: code/missionui/missionbrief.cpp
===================================================================
--- code/missionui/missionbrief.cpp	(revision 10757)
+++ code/missionui/missionbrief.cpp	(working copy)
@@ -1133,14 +1133,10 @@
 			brief_voice_play(Current_brief_stage);
 		}
 
-		if (gr_screen.res == 1) {
-			Max_brief_Lines = 110/gr_get_font_height(); //Make the max number of lines dependent on the font height. 225 and 85 are magic numbers, based on the window size in retail. 
-		} else {
-			Max_brief_Lines = 60/gr_get_font_height();
-		}
+		Max_brief_Lines = Brief_text_coords[gr_screen.res][3]/gr_get_font_height(); //Make the max number of lines dependent on the font height.
 
 		// maybe output the "more" indicator
-		if ( Max_brief_Lines < Num_brief_text_lines[0] ) {
+		if ( (Max_brief_Lines + Top_brief_text_line) < Num_brief_text_lines[0] ) {
 			// can be scrolled down
 			int more_txt_x = Brief_text_coords[gr_screen.res][0] + (Brief_max_line_width[gr_screen.res]/2) - 10;
 			int more_txt_y = Brief_text_coords[gr_screen.res][1] + Brief_text_coords[gr_screen.res][3] - 2;				// located below brief text, centered
@@ -1148,7 +1144,7 @@
 			gr_get_string_size(&w, &h, XSTR("more", 1469), strlen(XSTR("more", 1469)));
 			gr_set_color_fast(&Color_black);
 			gr_rect(more_txt_x-2, more_txt_y, w+3, h, GR_RESIZE_MENU);
-			gr_set_color_fast(&Color_red);
+			gr_set_color_fast(&Color_more_indicator);
 			gr_string(more_txt_x, more_txt_y, XSTR("more", 1469), GR_RESIZE_MENU);  // base location on the input x and y?
 		}
 	}
Index: code/missionui/missioncmdbrief.cpp
===================================================================
--- code/missionui/missioncmdbrief.cpp	(revision 10757)
+++ code/missionui/missioncmdbrief.cpp	(working copy)
@@ -730,14 +730,10 @@
 		Voice_good_to_go = 1;
 	}
 
-	if (gr_screen.res == 1) {
-		Max_cmdbrief_Lines = 166/gr_get_font_height(); //Make the max number of lines dependent on the font height. 225 and 85 are magic numbers, based on the window size in retail. 
-	} else {
-		Max_cmdbrief_Lines = 116/gr_get_font_height();
-	}
+	Max_cmdbrief_Lines = Cmd_text_wnd_coords[Uses_scroll_buttons][gr_screen.res][CMD_H_COORD]/(gr_get_font_height() + 1); //Make the max number of lines dependent on the font height, keeping in mind that we have an extra pixel between lines.
 
 	// maybe output the "more" indicator
-	if ( Max_cmdbrief_Lines < Num_brief_text_lines[0] ) {
+	if ( (Max_cmdbrief_Lines + Top_cmd_brief_text_line) < Num_brief_text_lines[0] ) {
 		// can be scrolled down
 		int more_txt_x = Cmd_text_wnd_coords[Uses_scroll_buttons][gr_screen.res][CMD_X_COORD] + (Cmd_text_wnd_coords[Uses_scroll_buttons][gr_screen.res][CMD_W_COORD]/2) - 10;
 		int more_txt_y = Cmd_text_wnd_coords[Uses_scroll_buttons][gr_screen.res][CMD_Y_COORD] + Cmd_text_wnd_coords[Uses_scroll_buttons][gr_screen.res][CMD_H_COORD] - 2;				// located below brief text, centered
@@ -745,7 +741,7 @@
 		gr_get_string_size(&w, &h, XSTR("more", 1469), strlen(XSTR("more", 1469)));
 		gr_set_color_fast(&Color_black);
 		gr_rect(more_txt_x-2, more_txt_y, w+3, h, GR_RESIZE_MENU);
-		gr_set_color_fast(&Color_red);
+		gr_set_color_fast(&Color_more_indicator);
 		gr_string(more_txt_x, more_txt_y, XSTR("more", 1469), GR_RESIZE_MENU);  // base location on the input x and y?
 	}
 
Index: code/missionui/missiondebrief.cpp
===================================================================
--- code/missionui/missiondebrief.cpp	(revision 10757)
+++ code/missionui/missiondebrief.cpp	(working copy)
@@ -1694,7 +1696,7 @@
 			break;
 
 		case TEXT_SCROLL_DOWN:
-			if (Max_debrief_Lines < Num_text_lines) {
+			if (Max_debrief_Lines < (Num_text_lines - Text_offset)) {
 				Text_offset++;
 				gamesnd_play_iface(SND_SCROLL);
 			} else {
@@ -2555,16 +2557,12 @@
 			break;
 	} // end switch
 
-	if (gr_screen.res == 1) {
-		Max_debrief_Lines = 450/gr_get_font_height(); //Make the max number of lines dependent on the font height. 225 and 85 are magic numbers, based on the window size in retail. 
-	} else {
-		Max_debrief_Lines = 340/gr_get_font_height();
-	}
+	Max_debrief_Lines = Debrief_text_wnd_coords[gr_screen.res][3]/gr_get_font_height(); //Make the max number of lines dependent on the font height.
 
-	if (Max_debrief_Lines < Num_text_lines) {
+	if ( (Max_debrief_Lines + Text_offset) < Num_text_lines ) {
 		int w;
 
-		gr_set_color_fast(&Color_red);
+		gr_set_color_fast(&Color_more_indicator);
 		gr_get_string_size(&w, NULL, XSTR( "More", 459));
 		gr_printf_menu(Debrief_text_wnd_coords[gr_screen.res][0] + Debrief_text_wnd_coords[gr_screen.res][2] / 2 - w / 2, Debrief_text_wnd_coords[gr_screen.res][1] + Debrief_text_wnd_coords[gr_screen.res][3], XSTR( "More", 459));
 	}
remove_more.patch (5,311 bytes)   

MageKing17

2014-06-06 04:46

developer   ~0015806

Decided offsets were stupid, found out computations for maximum lines were all completely screwed up in the first place. Rewrote to use existing constants instead of magic numbers, and tested to work. Thanks to niffiwan for removing those old patches.

niffiwan

2014-06-06 06:46

developer   ~0015807

Tested successfully with the same test mission I used previously. I also successfully tested the same mission using the BP2 fonts. Good work! :)

niffiwan

2014-06-06 06:46

developer   ~0015808

Fix committed to trunk@10758.

Related Changesets

fs2open: trunk r10758

2014-06-06 02:03

niffiwan


Ported: N/A

Details Diff
Fix mantis 3048 (From MageKing17)

Fix "more" & offsets in [cmd|de|]briefings
Affected Issues
0003048
mod - /trunk/fs2_open/code/globalincs/alphacolors.h Diff File
mod - /trunk/fs2_open/code/missionui/missionbrief.cpp Diff File
mod - /trunk/fs2_open/code/missionui/missioncmdbrief.cpp Diff File
mod - /trunk/fs2_open/code/missionui/missiondebrief.cpp Diff File

Issue History

Date Modified Username Field Change
2014-05-28 20:30 Scotty New Issue
2014-06-03 18:41 Lorric Note Added: 0015788
2014-06-04 03:16 Goober5000 Note Added: 0015789
2014-06-04 05:14 MageKing17 File Added: missiondebrief.cpp.patch
2014-06-04 05:14 MageKing17 File Added: dim_more.patch
2014-06-04 05:22 MageKing17 Note Added: 0015790
2014-06-04 05:43 niffiwan Assigned To => niffiwan
2014-06-04 05:43 niffiwan Status new => code review
2014-06-04 08:45 niffiwan Assigned To niffiwan =>
2014-06-04 22:08 MageKing17 File Added: dim_more2.patch
2014-06-04 22:10 MageKing17 Note Added: 0015792
2014-06-05 00:02 niffiwan Note Added: 0015793
2014-06-05 00:51 Goober5000 Note Added: 0015795
2014-06-05 01:47 MageKing17 File Added: remove_more.patch
2014-06-05 01:54 MageKing17 Note Added: 0015796
2014-06-05 11:11 niffiwan Note Added: 0015802
2014-06-06 03:24 MageKing17 File Added: remove_more2.patch
2014-06-06 03:24 MageKing17 Note Added: 0015805
2014-06-06 03:26 niffiwan File Deleted: missiondebrief.cpp.patch
2014-06-06 03:26 niffiwan File Deleted: dim_more.patch
2014-06-06 03:26 niffiwan File Deleted: dim_more2.patch
2014-06-06 03:26 niffiwan File Deleted: remove_more.patch
2014-06-06 04:36 niffiwan File Deleted: remove_more2.patch
2014-06-06 04:45 MageKing17 File Added: remove_more.patch
2014-06-06 04:46 MageKing17 Note Added: 0015806
2014-06-06 06:46 niffiwan Note Added: 0015807
2014-06-06 06:46 niffiwan Changeset attached => fs2open trunk r10758
2014-06-06 06:46 niffiwan Note Added: 0015808
2014-06-06 06:46 niffiwan Status code review => resolved
2014-06-06 06:46 niffiwan Resolution open => fixed
2014-06-09 01:22 Goober5000 Assigned To => MageKing17