refactor split_str_once

Rewrite `split_str_once` to be more robust, to handle Unicode, and to return length and position information rather than modifying the source string, taking the approach also used by `str_wrap_to_width`.  In combination with the new length variants of common string utility functions, this allows strings to be split without copying them.  In many cases, using the old design, strings were copied multiple times per frame in numerous places.

Includes unit tests to handle numerous usual and unusual cases.  All of the historical string splitting anomalies should now be fixed, including a bug in the retail algorithm where an exactly matching string followed by whitespace wasn't split properly.

A followup PR will refactor `split_str`.

Author: Goober5000

code/hud/hudmessage.cpp

@@ -104,7 +104,6 @@ static int Hud_mission_log_time2_coords[GR_NUM_RESOLUTIONS][2] = {
 #define SHOW_OBJS_BUTTON	4
 #define ACCEPT_BUTTON		5
 
-#define HUD_MSG_LENGTH_MAX		2048
 //#define HUD_MSG_MAX_PIXEL_W	439	// maximum number of pixels wide message display area is
 //#define HUD_MSG_MAX_PIXEL_W	619	// maximum number of pixels wide message display area is
 
@@ -293,47 +292,30 @@ void HudGaugeMessages::pageIn()
 
 void HudGaugeMessages::processMessageBuffer()
 {
-	int x, offset = 0;
-	size_t i;
-	char *msg;
-	char *split_str, *ptr;
-
-	for ( i = 0; i < HUD_msg_buffer.size(); i++ ) {
-		msg = new char [HUD_msg_buffer[i].text.size()+1];
-		strcpy(msg, HUD_msg_buffer[i].text.c_str());
-
-		ptr = strstr(msg, NOX(": "));
-		if ( ptr ) {
-			int sw;
-			gr_get_string_size(&sw, nullptr, msg, 1.0f, (ptr + 2 - msg));
-			offset = sw;
-		}
-
-		x = 0;
-		split_str = msg;
-
-		while ((ptr = split_str_once(split_str, Max_width - x - 7)) != nullptr) {		// the 7 is a fudge hack
-			// make sure that split went ok, if not then bail
-			if (ptr == split_str) {
-				break;
-			}
-
-			addPending(split_str, HUD_msg_buffer[i].source, x);
-			split_str = ptr;
+	for (auto &hud_msg: HUD_msg_buffer)
+	{
+		auto msg = hud_msg.text.c_str();
+
+		int x = 0, offset = 0;
+		auto ptr = strstr(msg, NOX(": "));
+		if (ptr)
+			gr_get_string_size(&offset, nullptr, msg, 1.0f, static_cast<size_t>(ptr + 2 - msg));
+
+		size_t split_len, split_next_pos;
+		do {
+			std::tie(split_len, split_next_pos, std::ignore) = split_str_once(msg, Max_width - x - 7);		// the 7 is a fudge hack
+			addPending(msg, split_len, hud_msg.source, x);
+			msg += split_next_pos;
 			x = offset;
-		}
-
-		addPending(split_str, HUD_msg_buffer[i].source, x);
-
-		delete[] msg;
+		} while (split_next_pos > 0);
 	}
 }
 
-void HudGaugeMessages::addPending(const char *text, int source, int x)
+void HudGaugeMessages::addPending(const char *text, size_t len, int source, int x)
 {
 	Assert(text != nullptr);
 
-	pending_messages.emplace(text, source, x);
+	pending_messages.emplace(SCP_string(text, len), source, x);
 }
 
 void HudGaugeMessages::scrollMessages()
@@ -829,50 +811,43 @@ void hud_scrollback_button_pressed(int n)
 // scroll height of the scrollback UI.
 void hud_initialize_scrollback_lines()
 {
-
 	Msg_scrollback_lines.clear();
 
-	if ((Msg_scrollback_vec.size() > 0) && HUD_msg_inited) {
+	if (!Msg_scrollback_vec.empty() && HUD_msg_inited) {
 
-		for (int j = 0; j < (int)Msg_scrollback_vec.size(); j++) {
-			line_node node_msg = Msg_scrollback_vec[j];
+		for (auto &node_msg: Msg_scrollback_vec) {
+			auto text = node_msg.text.c_str();
+			int max_width = Hud_mission_log_list2_coords[gr_screen.res][2];
 
 			int width = 0;
 			int height = 0;
-			gr_get_string_size(&width, &height, node_msg.text.c_str(), 1.0f, node_msg.text.length());
-
-			int max_width = Hud_mission_log_list2_coords[gr_screen.res][2];
-			if (width > max_width) {
-				char c_text[HUD_MSG_LENGTH_MAX];
-				strcpy_s(c_text, node_msg.text.c_str());
-
-				char* text = c_text;
-
-				char* split = split_str_once(text, max_width);
-				Msg_scrollback_lines.push_back({node_msg.time, The_mission.HUD_timer_padding, node_msg.source, node_msg.x, 1, node_msg.underline_width, text});
-
-				while (split != nullptr) {
-					text = split;
-					split = nullptr;
-					split = split_str_once(text, max_width);
-
-					int offset = 1;
-					if (split == nullptr)
-						offset = height / 3;
-
-					Msg_scrollback_lines.push_back({0, 0, node_msg.source, node_msg.x, offset, 0, text});
-				}
-			} else {
-				node_msg.y = height / 3;
-				Msg_scrollback_lines.push_back(std::move(node_msg));
-			}
+			bool first = true;
+			size_t start_pos = 0;
+			size_t split_len, split_next_pos;
+
+			do {
+				std::tie(split_len, split_next_pos, std::ignore) = split_str_once(text, max_width, std::string::npos, 1.0f, &width, &height);
+
+				Msg_scrollback_lines.emplace_back(
+					first ? node_msg.time : 0,
+					first ? node_msg.timer_padding : 0,
+					node_msg.source,
+					node_msg.x,
+					split_next_pos > 0 ? 1 : height / 3,		// y coordinate is 1 for every line except the last one
+					first ? node_msg.underline_width : 0,
+					node_msg.text.substr(start_pos, split_len)
+				);
+
+				text += split_next_pos;
+				start_pos += split_next_pos;
+				first = false;
+			} while (split_next_pos > 0);
 		}
 	}
 }
 
 void hud_scrollback_init()
 {
-
 	// pause all game sounds
 	weapon_pause_sounds();
 	audiostream_pause_all();
@@ -1050,8 +1025,7 @@ void hud_scrollback_do_frame(float  /*frametime*/)
 		int y = 0;
 		if (!Msg_scrollback_lines.empty() && HUD_msg_inited) {
 			int i = 0;
-			for (int j = 0; j < (int)Msg_scrollback_lines.size(); j++) {
-				line_node node_msg = Msg_scrollback_lines[j];
+			for (auto &node_msg: Msg_scrollback_lines) {
 				if ((node_msg.source == HUD_SOURCE_HIDDEN) || (i++ < Scroll_offset)) {
 					continue;
 

code/hud/hudmessage.h

@@ -120,7 +120,7 @@ class HudGaugeMessages: public HudGauge // HUD_MESSAGE_LINES
 
 	void clearMessages();
 	void processMessageBuffer();
-	void addPending(const char *text, int source, int x = 0);
+	void addPending(const char *text, size_t len, int source, int x = 0);
 	void scrollMessages();
 	void preprocess() override;
 	void render(float frametime, bool config = false) override;

code/mission/missionlog.cpp

@@ -439,12 +439,7 @@ int mission_log_get_count( LogType type, const char *pname, const char *sname )
 
 void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_color, const char *text, int flags = 0)
 {
-	// set the vector
-	entry->text.reset(vm_strdup(text));
-	entry->color = msg_color;
-	entry->x = x;
-	entry->line_offset = line_offset;
-	entry->flags = flags;
+	message_log_add_seg(entry, x, line_offset, msg_color, text, strlen(text), flags);
 }
 
 void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_color, const char *text, size_t len, int flags = 0)
@@ -457,66 +452,59 @@ void message_log_add_seg(log_text_seg *entry, int x, int line_offset, int msg_co
 	entry->flags = flags;
 }
 
-void message_log_add_segs(const char *source_string, int msg_color, int flags, SCP_vector<log_text_seg> *entry, bool split_string)
+void message_log_add_segs(const char *text, int msg_color, int flags, SCP_vector<log_text_seg> *entry, bool split_string)
 {
-	if (!source_string || !entry) {
+	if (!text || !entry) {
 		mprintf(("Why are you passing a NULL pointer to message_log_add_segs?\n"));
 		return;
 	}
-	if (!*source_string) {
-		return;
-	}
-        
-	// duplicate the string so that we can split it without modifying the source
-	char *dup_string = vm_strdup(source_string);
-	char *str = dup_string;
-	char *split = NULL;
 
 	if (split_string) {
 		int sanity_counter = 0;
 
 		while (true) {
 			if (X == ACTION_X) {
-				while (is_white_space(*str))
-					str++;
+				ignore_white_space(&text);
+			}
+			if (!*text) {
+				return;
 			}
 
-			if (P_width - X > 0) {
-				split = split_str_once(str, P_width - X);
+			int w;
+			auto [split_len, split_next_pos, forced] = split_str_once(text, P_width - X, std::string::npos, 1.0f, &w);
 
-				// if we couldn't actually split the string, try again on a new line
-				if (split == str) {
-					sanity_counter++;
-					if (sanity_counter > 5) {
-						Error(LOCATION, "Too many attempts to wrap a mission log line!  Get a coder!\nLine = %s", str);
-						break;
-					}
-				} else {
-					log_text_seg new_seg;
-					message_log_add_seg(&new_seg, X, Line_offset, msg_color, str, flags);
-					entry->push_back(std::move(new_seg));
-
-					if (!split) {
-						int w;
-						gr_get_string_size(&w, nullptr, str);
-						X += w;
-						break;
-					}
+			// we have more text to write (since we didn't return, above), so if we don't actually have enough room, try again on a new line
+			if (split_len == 0 || forced) {
+				split_next_pos = 0;
+				sanity_counter++;
+				if (sanity_counter > 5) {
+					Error(LOCATION, "Too many attempts to wrap a mission log line!  Get a coder!\nLine = %s", text);
+					break;
+				}
+			} else {
+				log_text_seg new_seg;
+				message_log_add_seg(&new_seg, X, Line_offset, msg_color, text, split_len, flags);
+				entry->push_back(std::move(new_seg));
+
+				if (split_next_pos == 0) {
+					X += w;
+					break;
 				}
 			}
 
 			X = ACTION_X;
 			Line_offset++;
-			str = split;
+			text += split_next_pos;
 		}
 	} else {
+		if (!*text) {
+			return;
+		}
+
 		log_text_seg new_seg;
-		message_log_add_seg(&new_seg, X, Line_offset, msg_color, str, flags);
+		message_log_add_seg(&new_seg, X, Line_offset, msg_color, text, flags);
 		entry->push_back(std::move(new_seg));
 	}
-
-	// free the buffer
-	vm_free(dup_string);
 }
 
 int mission_log_color_get_team(int msg_color)
@@ -691,8 +679,7 @@ void mission_log_init_scrollback(int pw, bool split_string)
 				Assert(!(entry.index & CARGO_NO_DEPLETE));
 
 				message_log_add_segs(XSTR("Cargo revealed: ", 418), LOG_COLOR_NORMAL, 0, &thisEntry.segments, split_string);
-				strncpy(text, Cargo_names[entry.index], sizeof(text) - 1);
-				message_log_add_segs(text, LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string);
+				message_log_add_segs(Cargo_names[entry.index], LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string);
 				break;
 
 			case LOG_CAP_SUBSYS_CARGO_REVEALED:
@@ -701,8 +688,7 @@ void mission_log_init_scrollback(int pw, bool split_string)
 
 				message_log_add_segs(entry.sname_display.c_str(), LOG_COLOR_NORMAL, 0, &thisEntry.segments, split_string);
 				message_log_add_segs(XSTR( " subsystem cargo revealed: ", 1488), LOG_COLOR_NORMAL, 0, &thisEntry.segments, split_string);
-				strncpy(text, Cargo_names[entry.index], sizeof(text) - 1);
-				message_log_add_segs(text, LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string);
+				message_log_add_segs(Cargo_names[entry.index], LOG_COLOR_BRIGHT, 0, &thisEntry.segments, split_string);
 				break;
 
 

code/mission/missiontraining.cpp

@@ -282,7 +282,6 @@ void HudGaugeDirectives::render(float  /*frametime*/, bool config)
 	int bx = x;
 	int by = y + fl2i(middle_frame_offset_y * scale);
 
-	char* second_line;
 	for (int i=0; i<end; i++) {
 		int ox = x + fl2i(text_start_offsets[0] * scale);
 		int oy = y + fl2i(text_start_offsets[1] * scale) + Training_obj_num_display_lines * fl2i(text_h * scale);
@@ -291,36 +290,37 @@ void HudGaugeDirectives::render(float  /*frametime*/, bool config)
 		int line_x_offset = 0;
 
 		color* c = &Color_normal;
-		char buf[256];
+		SCP_string buf;
 		if (!config && Training_obj_lines[i + offset] & TRAINING_OBJ_LINES_KEY) {
-			SCP_string temp_buf = message_translate_tokens(Mission_events[z].objective_key_text.c_str()); // remap keys
-			strcpy_s(buf, temp_buf.c_str());
+			buf = message_translate_tokens(Mission_events[z].objective_key_text.c_str()); // remap keys
 			c = &Color_bright_green;
 			line_x_offset = fl2i(key_line_x_offset * scale);
 		} else if (config){
 			switch (i) {
 			case 0:
-				strcpy_s(buf, "Destroy Enemies");
+				buf = "Destroy Enemies";
 				c = &Color_bright_white;
 				break;
 			case 1:
-				strcpy_s(buf, "Target hostile fighters");
+				buf = "Target hostile fighters";
 				c = &Color_bright_green;
 				line_x_offset = fl2i(key_line_x_offset * scale);
 				break;
 			case 2:
-				strcpy_s(buf, "Protect Friendlies");
+				buf = "Protect Friendlies";
 				c = &Color_bright_red;
 				break;
 			case 3:
-				strcpy_s(buf, "Escort Cruiser");
+				buf = "Escort Cruiser";
 				c = &Color_bright_blue;
 				break;
 			}
 		} else {
-			strcpy_s(buf, Mission_events[z].objective_text.c_str());
+			buf = Mission_events[z].objective_text;
 			if (Mission_events[z].count){
-				sprintf(buf + strlen(buf), NOX(" [%d]"), Mission_events[z].count);
+				buf += NOX(" [");
+				buf += std::to_string(Mission_events[z].count);
+				buf += NOX("]");
 			}
 
 			// if this is a multiplayer tvt game, and this is event is not for my team, don't display it
@@ -361,12 +361,8 @@ void HudGaugeDirectives::render(float  /*frametime*/, bool config)
 		}
 
 		// maybe split the directives line
-		second_line = split_str_once(buf, fl2i(max_line_width * scale), scale);
-
-		// if we are unable to split the line, just print it once
-		if (second_line == buf) {
-			second_line = nullptr;
-		}
+		size_t split_len, split_next_pos;
+		std::tie(split_len, split_next_pos, std::ignore) = split_str_once(buf.c_str(), fl2i(max_line_width * scale), std::string::npos, scale);
 
 		// blit the background frames
 		setGaugeColor(HUD_C_NONE, config);
@@ -376,7 +372,7 @@ void HudGaugeDirectives::render(float  /*frametime*/, bool config)
 
 		by += fl2i(text_h * scale);
 
-		if ( second_line ) {
+		if ( split_next_pos > 0 ) {
 
 			if (directives_middle.first_frame >= 0)
 				renderBitmap(directives_middle.first_frame, bx, by, scale, config);
@@ -387,14 +383,17 @@ void HudGaugeDirectives::render(float  /*frametime*/, bool config)
 		// blit the text
 		gr_set_color_fast(c);
 
-		renderString(ox + line_x_offset, oy, EG_OBJ1 + i, buf, scale, config);
+		renderString(ox + line_x_offset, oy, EG_OBJ1 + i, buf.c_str(), split_len, scale, config);
 
 		Training_obj_num_display_lines++;
 
-		if ( second_line ) {
+		if ( split_next_pos > 0 ) {
+			auto second_line = buf.c_str() + split_next_pos;
+			std::tie(split_len, split_next_pos, std::ignore) = split_str_once(second_line, fl2i(max_line_width * scale), std::string::npos, scale);
+
 			oy = y + fl2i(text_start_offsets[1] * scale) + Training_obj_num_display_lines * fl2i(text_h * scale);
 
-			renderString(ox+12, oy, EG_OBJ1 + i + 1, second_line, scale, config);
+			renderString(ox+12, oy, EG_OBJ1 + i + 1, second_line, split_len, scale, config);
 
 			Training_obj_num_display_lines++;
 		}