New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows_Message::ApplyTextInsertingCommands fix #1379

Merged
merged 1 commit into from Jul 1, 2018

Conversation

Projects
None yet
4 participants
@Albeleon
Member

Albeleon commented Jun 23, 2018

Problems:

  1. If you put in a message "\n[x]" whose name is also "\n[x]" it generates a loop problem.
  2. If "\n[x]" or "\v[x]" result in smaller characters (i.e. a/0) and they are the last characters in the message, it will generate an out-of-range error because the actor_replacement_start will be bigger than the replaced message.

Solutions:

  1. The actor_replacement_start should not be the end of the number, but code_start.
  2. actor_replacement_start should clamp with the size of the string after being replaced to avoid these problems.
  3. "start_code - 2", the -2 while it doesn't really bother it's pointless and the function is clearer without it (not a function that needs efficiency over simplicity).
@Ghabry

This comment has been minimized.

Member

Ghabry commented Jun 23, 2018

Could you reword the commit message so it doesn't start with "Problems:\n"? Because the first line is what the git summary shows.

Fixing loop problem in messages with the \n command, and out-of-range…
… index of messages.

Problems:
1. If you put in a message "\n[x]" whose name is also "\n[x]" it generates a loop problem.
2. If "\n[x]" or "\v[x]" result in smaller characters (i.e. a/0) and they are the last characters in the message, it will generate an out-of-range error because the actor_replacement_start will be bigger than the replaced message.

Solutions:
1. The actor_replacement_start should not be the end of the number, but code_start.
2. actor_replacement_start should clamp with the size of the string after being replaced to avoid these problems.
3. "start_code - 2", the -2 while it doesn't really bother it's pointless and the function is clearer without it (not a function that needs efficiency over simplicity).
@Ghabry

Ghabry approved these changes Jun 24, 2018

Works for me and doesn't trigger ASAN

@Ghabry Ghabry added this to the 0.5.4 milestone Jun 29, 2018

@carstene1ns carstene1ns merged commit 6023a58 into EasyRPG:master Jul 1, 2018

7 checks passed

Android (armeabi-v7a) Build finished.
Details
GCW0 Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details

@Albeleon Albeleon deleted the Albeleon:message branch Jul 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment