String Variables

[enewey]

implements all major maniacs string variable operations and modifiers, with the exception of the .sjis and .utf8 encoding modifiers for file reading/writing.

.asg & .cat
t[..]
v[..]
s[..]
.actor [a] .name
.skill [a] .name
.item [a] .name
.enemy [a] .name
.troop [a] .name
.terrain [a] .name
.element [a] .name
.state [a] .name
.anim [a] .name
.tileset [a] .name
.s [a] .name
.v [a] .name
.t [a] .name
.cev [a] .name
.class [a] .name
.anim2 [a] .name
.map [a] .name
.mev [a] .name
.member [a] .name
.actor [a] .desc
.skill [a] .desc
.item [a] .desc
.member [a] .desc
.cat
.ins
.rem
.rep
.exrep
.subs
.join
.file

.toNum
.getLen
.inStr
.exInStr
.exMatch
.split
.toFile
.popLine

also adds support for string comparators in conditional statements
.eq
.neq
.contains
.notContains

2k3 test project used for developing these features:
https://rpgmaker.net/users/narcodis/locker/easyrpg_testing.zip
proper unit testing could be added 😓

---

Web Player test: https://easyrpg.org/play/pr3051/?game=string-var

[Ghabry]

Jenkins: test this please <3

Thanks for this huge contribution!

You received an invite for the "Player Team". This whitelists you for the pull request builder (will auto-build your stuff).

Also add yourself and all other authors that contributed to this to the Authors.md if you havn't done already.

[Ghabry]

Not going to review this right now as is a bit too long xD.

But one remark already: I don't like the commit text. Could you prefix all of them with something like StringVar: to make it clear what the context is?

The easiest way to do this should be:

git rebase -i HEAD~23

Then change all the pick to reword.

After save and quit git will open a text editor for every single commit (so 23 times). Here prefix all the commits with something like StringVar: .

Then push: git push --force-with-lease origin.

Is a bit tedious but shouldn't take more than a few minutes.

[Ghabry]

Uploaded the string var test project to the web player: https://easyrpg.org/play/pr3051/?game=string-var (note that txt file loading won't work as the code for "async asset fetching" is not implemented currently)

[enewey]

Looks like String vars in a string pic isn't fully supported -- while you can display a stringvar through message codes (e.g. @pic[1].strpic { "\t[1]", ... } ), you cannot just plug in the string variable directly (e.g. @pic[1].strpic { t[1], ... }). Need to update string pics, and any command that takes a string parameter (like a filename).

[Ghabry]

except for StringPics (where this appears to be used alot(?)) I would prefer to have support for string-vars in the other commands as another PR.

Adding support to all other commands is alot of additional complexity and lots of more testing that nothing breaks. So not required for the MVP. ;)

[enewey]

I agree -- don't want to explode the scope of this. Isolating to supporting string pics sounds reasonable.

[Ghabry]

Only quickly looked through the code and these are the largest issues I'm currently seeing.

Sorry but text processing and encodings is tricky so we must think here of all the edge-cases as we are multi-platform, multi-language and Unicode aware, so a "lol, works in Japan for me" is not good enough.

All in all for your first PR (which solves a complex problem) this is already pretty good work :).

[Ghabry]

This must be checked (same for index returned by InStr):

What kind of encoding is Maniac Patch using here?

Is this the length in bytes or in codepoints? When it is the length in bytes is it in local encoding or in UTF-8?

[enewey]

When testing the string My name is Đức:
Assigned to a string directly, length is 14.
When read from a file specified as utf8, length is 14.
When read from a file specified as ansi, length is 17.

[enewey]

With the PR as it is, the player does not behave the same way.
With files read as utf8, we are saying its length is 17
With files read as ansi, we are saying its length is 17
And when assigned directly to a string, we are saying its length is 15... I believe this is because tpc doesn't allow the ứ character (either due to my computer's locale or tpc itself, no idea).

[enewey]

Looks like BingShan recently released an update to tpc on his private discord that does not encode the text-editor as shift-jis, so I may be able to test this a little more comprehensively... I'll check this out.

[Ghabry]

std::stoi is problematic because it throws an exception when conversion fails and we have exceptions disabled = crash

If you need no error checking (error is 0, if this matches Maniac Patch???) use atoi and if you need error checking and different bases use std::strtoimax https://en.cppreference.com/w/cpp/string/byte/strtoimax

[enewey]

Did not know this about std::stoi, good note! I will look into the alternatives suggested.

[jetrotal]

Ghabry asked to list the issues pointed at rm2k3 channel:

discord issue 1:

---

discord issue 2:

t[1] .asg s[1]

---

discord issue 3:

---

discord issue 4:

@loop 20 {
    v[1] += 1
    t[2] .asg v[1]
    t[1] .asg  .cat t[2], "
", t[1]
    
}

@pic[1].strpic {
    "\t[1]"
    .pos 160, 120 .center
    .size 0, 0    .chromakey 1
    .scale 100
    .trans 0
    .rgbs 100, 100, 100, 100
    .font "", 12
    .spacing 0, 4
    .skin "" .stretch
    .mapLayer 7
    .eraseWhenTransfer
    .affectedByFlash
    .affectedByShake
}

this last one may have been fixed already.

[Ghabry]

Looks pretty good, I think we are 98% there :)

[Ghabry]

I rebased the PR but I cannot push to the branch (because it is master I guess?).

Will likely close this PR later and create my own to resolve the last pending issues.

Oh and thanks again for figuring this all out. This looked like a huge afford, the stuff I have to fix is simple compared to it ^^

[enewey]

Have resolved most issues, still a couple hanging threads to resolve.

An issue has cropped up with displaying string pictures, though. Seems to be related to some code changes I had to accommodate on rebase in the PendingMessage class. It appears when a string is assigned a string containing a string param (i.e. t[1] .asg "\t[2]"), it will strip out all newline characters from the original string. Not sure what else it might be stripping out.

Maniacs rpg_rt:

EasyRPG player:

Player also appears to incorrectly(?) color text when maniacs does not, but I don't know how concerned I am about that particular thing.

For the newline stripping piece, I will try to find time to dig into the PendingMessage class in more depth to deduce why this is happening.

[Ghabry]

I have a hard time going through the tests because I'm already stuck for a long time at the first one: Bottom left. Just this bottom left test took me already 90 minutes to check what is going in...

First discovery: Maniac changed how unknown command codes are parsed. E.g. a message with \A[0] \B[1] \e[2] is rendered as [0] [1] [2] by the Player. This is also what RPG_RT does, Maniac changed this and it renders \A[0] \B[1] \e[2] instead. I believed for 45 minutes that this is a player bug until I finally started an original RPG_RT to notice that it is a Maniac change -_-

---

Top Maniac, Bottom Player:

I wouldn't say that we are wrong here. This is just how it worked in original RPG_RT. Also is an edge-case nobody will usually encounter. Notice the missing \ at the last line before the s[1]. The Maniac parser has some bug here and is inconsistent.

Imo test passed ✔️

---

Looks like what we need for \t in a message box is to evaluate the expression? Though enabling the evaluation will also enable it for the String Picture because they share the same code. Is this a problem? Where is the y in Maniac coming from?

Test failed ➖

---

Upon more testing there seems to be in general a difference in how the message box resolves codes:

When you call Hero 1 \n[2] and do in code \n[1] then Maniac will replace this with the name of the 2nd hero while RPG_RT and EasyRPG render [2].

[Ghabry]

The 2nd test at the bottom row is also completely different.

Left is Maniac, Right is Player. Am I using the wrong version of Maniac patch? I use the RPG_RT.exe provided in the ZIP archive of the test game.

---

Sorry I cannot continue this review under these circumstances. My time is limited and something is completely broken.

[jetrotal]

Some direct comparations EasyRPG x Maniacs_RT (bigger windows being maniacs).
I may repeat the reports you guys gave:

Read PNG:

Uncommon Characters:

var, name, string, switch:

Database stuff (error only on easyRPG - Seems correct?):

"\t[1]" fails to read new lines, but interprets \c sintaxes:

[Ghabry]

About the remaining issues according to the screenshots:

• Read PNG: I guess the difference is that we read in "Binary mode" and Maniac_RT in "Text mode". Text mode is not portable across operating systems so I would suggest to ignore this edge case (actually with our implementation it should be possible to read a PNG in "utf-8" mode and write the PNG in "utf-8" mode and it should survive 😅)
• Has Patch Uncommon Characters: I implemented this now locally and I get UTF-8 17 and ANSI 14, so the opposite of Maniac. The problem here is that our Player uses UTF-8 internally and Maniac_RT likely the local codepage (as the old RPG_RT). That an implementation detail and imo not worth to fix.
• var, name, string, switch: This is a difference in how we evaluate variables in Messages and String Pics. Fixing this will break it in the other case though... So I would be fine with keeping this broken until a later PR.
• "\t[1]" fails to read new lines, but interprets \c sintaxes: Another implementation difference of Messages vs. String Pics: String Pics evaluate \n while in Messages they don't.

The last two cannot be fully implemented because we use shared code for both messages and string pics compared to Maniacs which seems to use 2 implementations......

[Ghabry]

@enewey I figured out solutions for most of the remaining problems. Just need to clean it up a bit. Will send you a patch on Sunday or Monday that will resolve them.

[jetrotal]

one thing we forgot to take a look is loading text files with webplayer

[Ghabry]

@enewey Creating commits was easier than explaining but I cannot push to your master branch (permission denied).

Please grab my commits manually:

git remote add ghabry https://github.com/ghabry/easyrpg-player
git fetch ghabry
git rebase ghabry/strvar --autostash --committer-date-is-author-date

Now you have my commits and you can push.

[Ghabry]

@jetrotal In case you already want to retest here is an emscripten instance with my changes:

http://server.mastergk.de:8000/easyrpg-player.html

[Ghabry]

This PR is already complex enough and it is a very solid implementation of String Vars. Thanks again for this huge contribution.

There is only nitpicky stuff left (e.g. lack of unit tests, lots of copying instead of string view, code formatting in some parts) but this can be all addressed later.