Skip to content

Fix #3403: Broken text input args#3543

Merged
duncanspumpkin merged 3 commits intoOpenLoco:masterfrom
duncanspumpkin:fix3403TextInput
Dec 16, 2025
Merged

Fix #3403: Broken text input args#3543
duncanspumpkin merged 3 commits intoOpenLoco:masterfrom
duncanspumpkin:fix3403TextInput

Conversation

@duncanspumpkin
Copy link
Copy Markdown
Contributor

No description provided.


FormatArguments args{};
args.skip(4);
args.skip(8);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must have been broken for quite few releases!


auto commonArgs = FormatArguments::common();
std::memcpy(_formatArgs.data(), commonArgs.getBufferStart(), commonArgs.getLength());
std::memcpy(_formatArgs.data(), commonArgs.getBufferStart(), std::min(commonArgs.getCapacity(), _formatArgs.capacity()));
Copy link
Copy Markdown
Contributor Author

@duncanspumpkin duncanspumpkin Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commonArgs.getLength() will always be zero as we just created it. Ideally we should be passing in the args then we would have a real length value. I broke this part in the refactoring.

// copy the existing args
FormatArgumentsBuffer formatArgsBuffer2 = _formatArgs;
auto args2 = FormatArguments(formatArgsBuffer2);
// copy and shuffle the existing args
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was also broken in the refactoring as I didn't notice it was shuffling the args from 8 to 2.

@@ -89,7 +89,7 @@ namespace OpenLoco::Ui::Windows::TextInput
window->initScrollWidgets();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future refactoring this function should really take messageArgs, titleArgs, valueArgs. At present valueArgs is passed in but message and title args is passed via the global common format arguments and then we do this horrible shuffling to split them into two.

@duncanspumpkin duncanspumpkin added this to the v25.12 milestone Dec 16, 2025
Co-authored-by: Aaron van Geffen <aaron@aaronweb.net>
@duncanspumpkin duncanspumpkin enabled auto-merge (squash) December 16, 2025 08:54
@duncanspumpkin duncanspumpkin linked an issue Dec 16, 2025 that may be closed by this pull request
@duncanspumpkin duncanspumpkin merged commit df07880 into OpenLoco:master Dec 16, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pre-existing name of town not displayed in text input window

2 participants