Preparations for RPG_RT - Player regression testing

[Ghabry]

With PRs such as #2208 where normal code reviews fail we finally need an automatic regression tester.

@fmatthew5876 I could need your help here. I ask you to provide two things. It would help me when you can contribute them (best would be as a seperate PR, so I can use it in both master and your character PR)

My idea is the following:

1. Add a "unit test" mode to Player which saves the game each frame
2. Replay the input in RPG_RT through injected DLL while saving the game each frame (for synchronization the Game_System.frame_counter is used)
3. Compare the save files
4. .....
5. PROFIT

---

TODO

• Invent a better input recording format [1] @Ghabry
• Add a new SavegameManager class that provides a Save(int save_id) function. @fmatthew5876 can you help with this? A Load function isn't needed yet but I need a globally accesible Save. [2]
• Write a unit test mode that saves the game each frame (easy to add when Save() exists)
• Write a DynRpg plugin that can replay input and save each frame @Ghabry
• Write an automatic difftool (python based?), could parse lcf2xml output and skip chunks that usually don't match (see [2])
• Record some tests

Known issues

• DynRPG only writes up to Save99.lsd, needs some trickery (file renaming) to record longer
• The recording desyncs when there is a teleport. Is off by 5 frames?!

[1] My suggestion: 6531fd8 The important lines are the "F" lines, the format is game_system_frame,keys..... A frame can occur multiple times because Game_System frames are reset in some cases, but this is no problem.

H EasyRPG Player Recording
V 2
D 2020-08-23-23-27-37
F 45,DECISION
F 0,DECISION
F 5,RIGHT
F 6,RIGHT
[...] <- A line is only written when there was an input in this frame
F 28,RIGHT,UP
F 29,RIGHT,UP
F 30,RIGHT,UP

[2]

Notes about savegame incompatibility

• title.timestamp - this varies (obviously)
• system.scene is 0 on the map
• easyrpg.version - only we write this

Strange observations

• system.save_count when using dynrpg save this is always 0
• system.save_slot when using dynrpg save this is always 1

Incompatible save storage

• Player stores <saveactor><status>0 0 0 0 0 0 0 0 0 0</status>, but RPG_RT stores <status></status> @fmatthew5876 can you fix this? I guess RPG_RT only writes numbers until it reaches all zero.

Lower priority

• Write a program that injects a DLL in RPG_RT that is DynRPG independent and provides the required features

[Ghabry]

Further idea via IRC:

The savegame could be a concatenated file.

Maybe add a --unittest <NAME> flag. When set the Player appends a save each frame to a <NAME> file. NOTE: Don't write while on the title screen. @fmatthew5876

[mateofio]

I looked into the states thing, unfortunately it's a bit tricky. RPG_RT actually resizes the states array the first time an actor is added to your party, regardless of whether they have states or not.

Ultimately, I think this and other related things depends on EasyRPG/liblcf#371

That is we should get rid of these setup and fixup functions. Liblcf should just read and present whatever in raw form, and then we do any fixing and adjusting in player.

It's on my todo list. But want to get the string/array optimization stuff in first to avoid rebase problems.

[mateofio]

Add a new SavegameManager class that provides a Save(int save_id) function. @fmatthew5876 can you help with this? A Load function isn't needed yet but I need a globally accesible Save. [2]

Ultimately my objective for this one is part of #1954

That is if we create a new Game_Data singleton which is responsible for initializing, storing, and tearing down all the game state, it can have a Save(slot) function which saves it all.

The problem of course is this has a dependency on #2208, but we want to have this now so we can test #2208 !

Therefore as a temporary solution I can just rip the saving functionality out of Scene_Save and make it globally accessible so we're not blocked.

Notes about savegame incompatibility

We're going to need some kind of "black list" config file which contains the set of chunks to ignore. This would either be used to omit chunks from the saves (since they not intended to be loaded, this is fine), or later in the pipeline as part of the diffing tool. I would do it wherever it is easiest and most convenient as a first step.

Write an automatic difftool (python based?), could parse lcf2xml output and skip chunks that usually don't match (see [2])

For a long replay this could be slow. Maybe we can binary diff the save files first, and only if they don't match, generate xml and diff again to show it in a human readable fashion?

Anyway, I would put such optimization concerns off until after the first version.

[Ghabry]

Therefore as a temporary solution I can just rip the saving functionality out of Scene_Save and make it globally accessible so we're not blocked.

Okay but please also provide a Filesystem_Inputstream version for handling the "append mode".

or later in the pipeline as part of the diffing tool. I would do it wherever it is easiest and most convenient as a first step.

Diffing tool is easier for now. Performance doesn't matter yet for the prototype.

For a long replay this could be slow.

Yeah for match the hash is fine but we are not there yet.

[Ghabry]

save_count and save_slot is handled by the File scene in RPG_RT. This explains the wrong values.

GetLocalTime can be hooked to modify title.timestamp.

[Ghabry]

The DynRPG plugin is finished https://github.com/EasyRPG/regression-tests/tree/master/tools/regtest-plugin

Usage:

wine rpg_rt.exe TestPlay X Window --replay-input input.log --regression-test save-concat.lsd

[Ghabry]

Here a python script for extracting all saves from a concatenated save. Unfortunately there is no size field after the header so I have to process all top chunks (advantage: Better detection of corrupted files)

This was the difficult part of the script imo. Missing is only: Read two of these files, then hash compare LSD by LSD and when they mismatch extract them, convert them with lcf2xml and output a diff.

import sys

chunks = list(range(0x64, 0x72+1))

save_data = []
cur_save_data = bytearray(b"")

def read(stream, count):
	b = stream.read(count)
	cur_save_data.extend(b)
	return b

def read_ber(stream):
	value = 0;
	temp = 0x80;
	while temp & 0x80:
		value <<= 7
		temp = int.from_bytes(read(stream, 1), byteorder='little')
		value |= (temp & 0x7F)
	return value

def read_save(stream):
	saves = len(save_data) + 1

	if read(f, 11).decode("ascii") != "LcfSaveData":
		raise ValueError(f"Save {saves} corrupted: Expected LcfSaveData")

	while (True):
		chunk = read_ber(f)
		if chunk not in chunks:
			if chunk == 0xB:
				# Start of new save
				del cur_save_data[-1]
				return True
			elif chunk == 200:
				# Likely not what you want for a compare
				print(f"Warning: Save {saves} contains EasyRPG chunk")
			elif chunk == 0:
				if not stream.read(1):
					# EOF
					return False
			raise ValueError(f"Save {saves} corrupted: Bad chunk {chunk}")
		chunk_size = read_ber(f)
		read(f, chunk_size)

with open(sys.argv[1], "rb") as f:
	if read(f, 1) != b"\x0b":
		raise ValueError(f"Not a save file")
	while (read_save(f)):
		save_data.append(cur_save_data)
		cur_save_data = bytearray(b"\x0b")
	save_data.append(cur_save_data)

print(f"Found {len(save_data)} savegames")

# Example: Extract them
for i, save in enumerate(save_data):
	with open(f"Save{i+1}.lsd", "wb") as f:
		f.write(save)

[Ghabry]

A problem with the savegame hashing: The hash varies but lcf2xml says "they are identical" (so I can still work with them, but is suspicious and not suitable for mass-testing -> slow).

Here are the two files:
same.zip

---

Worth investigating:

Another logic difference but not really testable because RNG related:
RPG_RT keeps "encounter calling" on TRUE even when there are no encounters on the map. Player resets the flag.
Maybe this means the entering an area while this is T will force an encounter when the area is touched? Needs testing.

[Ghabry]

There appears to be an input lag of one frame or my hook is bad.
For consistency I must use a custom hook point. Currently in RPG_RT this is OnFrame but in Player it is right before IncFrame is called. In both engines the hook must be exactly at the same position.

---

RPG_RT:

1. Calls sub_48FE98 <- This calls AsyncKeyState
2. scene update loop
3. inc frame counter
4. update the inputs
5. switch branching

So when hooking GetAsyncKeyState the frame counter is one behind. Maybe inc the frame counter to increase accuracy?