0003102: Fix to make show-subtitle-text positioning consistent on MP clients (code included)

View Issue Details [ Jump to Notes ] [ Related Changesets ]

[ Issue History ] [ Print ]

Project

Category

View Status

Date Submitted

Last Update

0003102

FSSCP

SEXPs

public

2014-09-02 20:47

2014-10-06 21:55

Reporter

Parias

Assigned To

Parias

Priority

normal

Severity

major

Reproducibility

always

Status

resolved

Resolution

fixed

Product Version

3.7.1

Target Version

3.7.2

Fixed in Version

3.7.2

Summary

0003102: Fix to make show-subtitle-text positioning consistent on MP clients (code included)

Description

Right now, show-subtitle-text has a bit of a quirk where it will send the raw x_pos and y_pos (and width) values *AFTER* the host has already calculated them (based on x_pct and y_pct, which are passed as SEXP arguments from the mission). This leads to issues if:

1) The host is running a different screen resolution than client systems (text will appear offset / misaligned from what the host sees)

2) The host is a dedicated / standalone server (text will just get all jumbled up in a corner because the values used for screen resolution percentage processing will come up null, or zero or something)

In either case, it's a bit problematic if you're expecting the text to line up in a certain way for all players. The attached .patch is a code fix to resolve this.

Additional Information

As always, I'm pretty new to modifying the code and so I'm not sure if there's a better way to do this - treat this change with all due skepticism.

But anyways, the logic is that instead of having sexp_show_subtitle_text do all of the grunt work and then send the results (work out x_pos, y_pos, and width and send those), it'll instead send the original x_pct, y_pct, and width_pct values as received from the SEXP via the multiplayer SEXP packet system; the multi_sexp_show_subtitle_text void will then receive the percent values and do it's own calculation on the client end based on the client's own screen resolution.

Tested this with a few configs just this last weekend and this appears to lead to much better results.

Tags

No tags attached.

Attached Files

sexp.cpp-subtitleclientposition.patch (1,913 bytes) 2014-09-02 20:47

Index: code/parse/sexp.cpp
===================================================================
--- code/parse/sexp.cpp	(revision 10990)
+++ code/parse/sexp.cpp	(working copy)
@@ -21079,8 +21108,8 @@
 	Subtitles.push_back(new_subtitle);
 
 	multi_start_callback();
-	multi_send_int(x_pos);
-	multi_send_int(y_pos);
+	multi_send_int(x_pct);
+	multi_send_int(y_pct);
 	multi_send_string(text);
 	multi_send_float(display_time);
 	multi_send_float(fade_time);
@@ -21090,23 +21119,23 @@
 	multi_send_int(fontnum);
 	multi_send_bool(center_x);
 	multi_send_bool(center_y);
-	multi_send_int(width);
+	multi_send_int(width_pct);
 	multi_send_bool(post_shaded);
 	multi_end_callback();
 }
 
 void multi_sexp_show_subtitle_text()
 {
-	int x_pos, y_pos, width=0, fontnum;
-	char text[TOKEN_LENGTH];
+	int x_pct, y_pct, width_pct, fontnum;
+	char text[256];
 	float display_time, fade_time=0.0f;
 	int red=255, green=255, blue=255;
 	bool center_x=false, center_y=false;
 	bool post_shaded = false;
 	color new_color;
 
-	multi_get_int(x_pos);
-	multi_get_int(y_pos);
+	multi_get_int(x_pct);
+	multi_get_int(y_pct);
 	multi_get_string(text);
 	multi_get_float(display_time);
 	multi_get_float(fade_time);
@@ -21116,11 +21145,16 @@
 	multi_get_int(fontnum);
 	multi_get_bool(center_x);
 	multi_get_bool(center_y);
-	multi_get_int(width);
+	multi_get_int(width_pct);
 	multi_get_bool(post_shaded);
 
 	gr_init_alphacolor(&new_color, red, green, blue, 255);
 
+	// calculate pixel positions
+	int x_pos = (int)(gr_screen.max_w * (x_pct / 100.0f));
+	int y_pos = (int)(gr_screen.max_h * (y_pct / 100.0f));
+	int width = (int)(gr_screen.max_w * (width_pct / 100.0f));
+
 	// add the subtitle
 	subtitle new_subtitle(x_pos, y_pos, text, NULL, display_time, fade_time, &new_color, fontnum, center_x, center_y, width, 0, post_shaded);
 	Subtitles.push_back(new_subtitle);

sexp.cpp-subtitleclientposition.patch (1,913 bytes) 2014-09-02 20:47

sexp.cpp-subtitleclientposition-new.patch (1,947 bytes) 2014-10-05 20:42

Index: code/parse/sexp.cpp
===================================================================
--- code/parse/sexp.cpp	(revision 11099)
+++ code/parse/sexp.cpp	(working copy)
@@ -21078,8 +21078,8 @@
 	Subtitles.push_back(new_subtitle);
 
 	multi_start_callback();
-	multi_send_int(x_pos);
-	multi_send_int(y_pos);
+	multi_send_int(x_pct);
+	multi_send_int(y_pct);
 	multi_send_int (message_index);
 	// only send the text if it is not a message. If it is a message, we've already sent the index anyway. 
 	if (message_index == -1) {
@@ -21093,14 +21093,14 @@
 	multi_send_int(fontnum);
 	multi_send_bool(center_x);
 	multi_send_bool(center_y);
-	multi_send_int(width);
+	multi_send_int(width_pct);
 	multi_send_bool(post_shaded);
 	multi_end_callback();
 }
 
 void multi_sexp_show_subtitle_text()
 {
-	int x_pos, y_pos, width=0, fontnum, message_index = -1;
+	int x_pct, y_pct, width_pct, fontnum, message_index = -1;
 	char text[MESSAGE_LENGTH];
 	float display_time, fade_time=0.0f;
 	int red=255, green=255, blue=255;
@@ -21108,8 +21108,8 @@
 	bool post_shaded = false;
 	color new_color;
 
-	multi_get_int(x_pos);
-	multi_get_int(y_pos);
+	multi_get_int(x_pct);
+	multi_get_int(y_pct);
 	multi_get_int(message_index); 
 	if (message_index == -1) {
 		multi_get_string(text);
@@ -21126,11 +21126,16 @@
 	multi_get_int(fontnum);
 	multi_get_bool(center_x);
 	multi_get_bool(center_y);
-	multi_get_int(width);
+	multi_get_int(width_pct);
 	multi_get_bool(post_shaded);
 
 	gr_init_alphacolor(&new_color, red, green, blue, 255);
 
+	// calculate pixel positions
+	int x_pos = (int)(gr_screen.max_w * (x_pct / 100.0f));
+	int y_pos = (int)(gr_screen.max_h * (y_pct / 100.0f));
+	int width = (int)(gr_screen.max_w * (width_pct / 100.0f));
+
 	// add the subtitle
 	subtitle new_subtitle(x_pos, y_pos, text, NULL, display_time, fade_time, &new_color, fontnum, center_x, center_y, width, 0, post_shaded);
 	Subtitles.push_back(new_subtitle);

sexp.cpp-subtitleclientposition-new.patch (1,947 bytes) 2014-10-05 20:42

Relationships

Relationships

Notes
~0016267 Parias (developer) 2014-09-02 21:00	Doh - I just realized the .patch also has the change for char text[TOKEN_LENGTH] in multi_sexp_show_subtitle_text() rolled in (changed from TOKEN_LENGTH to 256 as per 0003077) so just an FYI as I think that just got committed to trunk. I'll upload a fresh .patch without it if desired.

~0016298 Goober5000 (administrator) 2014-09-24 22:10	The patch looks good, but it gets rejected on current trunk. Could you upload a patch against the most recent revision?

~0016327 Parias (developer) 2014-10-05 20:43	Sorry for the delay Goob - I've fired over an updated .patch (and re-tested against the latest base trunk revision to ensure it goes in). Let me know if there are any more problems.

~0016328 Goober5000 (administrator) 2014-10-06 21:55	Patch committed. Marking as fixed.

Notes

Related Changesets
fs2open: trunk r11108 Timestamp: 2014-10-06 22:32:35 Author: Goober5000 Ported: N/A [ Details ] [ Diff ]	Parias's patch for Mantis 0003102 (make show-subtitle-text positioning consistent on MP clients)
	mod - /trunk/fs2_open/code/parse/sexp.cpp	[ Diff ] [ File ]

Related Changesets

Date Modified	Username	Field	Change
Issue History
2014-09-02 20:47	Parias	New Issue
2014-09-02 20:47	Parias	File Added: sexp.cpp-subtitleclientposition.patch
2014-09-02 21:00	Parias	Note Added: 0016267
2014-09-16 17:22	Echelon9	Status	new => code review
2014-09-24 22:10	Goober5000	Note Added: 0016298
2014-10-05 20:42	Parias	File Added: sexp.cpp-subtitleclientposition-new.patch
2014-10-05 20:43	Parias	Note Added: 0016327
2014-10-06 21:54	Goober5000	Changeset attached	=> fs2open trunk r11108
2014-10-06 21:55	Goober5000	Note Added: 0016328
2014-10-06 21:55	Goober5000	Assigned To	=> Parias
2014-10-06 21:55	Goober5000	Resolution	open => fixed
2014-10-06 21:55	Goober5000	Fixed in Version	=> 3.7.2
2014-10-06 21:55	Goober5000	Target Version	=> 3.7.2
2014-10-06 21:55	Goober5000	Status	code review => resolved

Issue History