View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003102 | FSSCP | SEXPs | public | 2014-09-03 00:47 | 2014-10-07 01: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. | ||||
|
|
sexp.cpp-subtitleclientposition.patch (1,913 bytes)
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); |
|
|
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. |
|
|
The patch looks good, but it gets rejected on current trunk. Could you upload a patch against the most recent revision? |
|
|
sexp.cpp-subtitleclientposition-new.patch (1,947 bytes)
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);
|
|
|
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. |
|
|
Patch committed. Marking as fixed. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-09-03 00:47 | Parias | New Issue | |
| 2014-09-03 00:47 | Parias | File Added: sexp.cpp-subtitleclientposition.patch | |
| 2014-09-03 01:00 | Parias | Note Added: 0016267 | |
| 2014-09-16 21:22 | Echelon9 | Status | new => code review |
| 2014-09-25 02:10 | Goober5000 | Note Added: 0016298 | |
| 2014-10-06 00:42 | Parias | File Added: sexp.cpp-subtitleclientposition-new.patch | |
| 2014-10-06 00:43 | Parias | Note Added: 0016327 | |
| 2014-10-07 01:54 | Goober5000 | Changeset attached | => fs2open trunk r11108 |
| 2014-10-07 01:55 | Goober5000 | Note Added: 0016328 | |
| 2014-10-07 01:55 | Goober5000 | Assigned To | => Parias |
| 2014-10-07 01:55 | Goober5000 | Resolution | open => fixed |
| 2014-10-07 01:55 | Goober5000 | Fixed in Version | => 3.7.2 |
| 2014-10-07 01:55 | Goober5000 | Target Version | => 3.7.2 |
| 2014-10-07 01:55 | Goober5000 | Status | code review => resolved |
