Skip to content
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

Support bit perfect reading and writing LDB, LMT, and LMU #242

Merged
merged 22 commits into from Oct 29, 2018

Conversation

Projects
None yet
4 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Sep 30, 2018

This series of changes supports bitperfect reading and writing of RPG_RT.ldb binary databases. It supports 2k and 2k3 mode.

Closes #234 and closes #229.

This one depends on all my other PR's.

I've tested it with the following:

  • An empty 2k3 database
  • The default New project database for 2k3
  • An empty 2k database
  • The default new project database for 2k
  • HH3 (2k game ~ 16MB RPG_RT.ldb)

Please if anyone has time. Checkout this branch, download some rm2k/rm2k3 games and test it against their databases. Let me know if any game doesn't come through perfectly.

fmatthew5876 added some commits Sep 29, 2018

Add presentifdefault flag
Fix Terms: battle_start/miss/status/row/order/wait_on/wait_off are 2k3 only
PassThrough Battlecommands Unknown_09
Default it to -1. If it's present in db we read, we set it
to some value and then it'll be written out. If it's not present,
it'll stay -1 and then the default handler will not write it.
Support RPG::Terms 0x1/0x3 defaulting semantics
RPG::Terms 0x3 if empty string will be present in a 2k database
but not a 2k3 database. This appears a one-off situation so it
is hacked in for now..

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch from 214ca3a to 332de99 Sep 30, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

Great. I will shoot this at my local game list and the rmarchiv.tk database (>1000 games) later.

Show resolved Hide resolved CMakeLists.txt

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch from 332de99 to a5294f5 Sep 30, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Sep 30, 2018

.flow would likely fail to convert cleanly as it has a 2k3 database but we detect it as 2k. I don't think there is anything we can do in situations like that.

Regarding Makefile.am, is maintaining autotools support really still needed now that we have Cmake?

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Sep 30, 2018

As said, we currently use the autotools for the distribution archives and some packagers will also depend on them (i.e. macOS homebrew, some GNU/Linux distributions, platforms without cmake support).

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

but what we will remove soon are the Visual Studio project files which are by far the worst to maintain with there XML bloat.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch from a5294f5 to 22dcc27 Sep 30, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

@fmatthew5876
Question about LDB read-write: Did you test this with LDB->XML->LDB or LDB read->LDB write?

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Sep 30, 2018

@fmatthew5876
Question about LDB read-write: Did you test this with LDB->XML->LDB or LDB read->LDB write?

Not yet. I'll look into this later today.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

Okay then I maybe misunderstood what I'm supposed to test.
Because you wrote "bit perfect reading/writing" I expect that a file loaded with LDB_Reader::Load followed by LDB_Reader::Save matches but when I diff e.g. your HH LDB-Database I get "binary files differ".

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Sep 30, 2018

Sorry I didn't answer that well. I've only tested LDB -> LDB. I haven't done xml yet.

Which version of HH do you have? I tested it on the latest one in my github.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

Okay, what I did: Compiled liblcf on branch "ldb_enums2", then compiled the following program:

#include <cstring>
#include <string>

#include "ldb_reader.h"

int main(int argc, char** argv)
{
        std::string infile = argv[1];
        std::string outfile = argv[2];
        LDB_Reader::Load(infile, "1252");
        LDB_Reader::Save(outfile, "1252");

        return 0;
}

Then I convert via this tool and run "diff RPG_RT.ldb RPG_RT2.ldb"

binary files dffer

Also tested it with your HH3 ldb

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Sep 30, 2018

My test driver is this:

https://github.com/fmatthew5876/hh3-rm2k/blob/fixes-2.1/dev/autodb/main.C

without the call to doWeaponCopies()

One difference I see is I'm passing "" as the encoding, where you're using "1252"

EDIT: also I forgot to mention. The hh3 version I tested on is in the fixes-2.1 branch. Not master.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

okay it doesn't matter if I do LDB->XML->LDB or LDB->LDB, both give the same results.

With your hh3 version I get "identity" but for anything else I fire at the converter the result differs.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 30, 2018

For my test data I get a perfect match for:

For 2k: 2 out of 1110
For 2k3: 42 out of 435

Which is a good start imo

Fix RPG::Parameters size bug
RPG::Parameters was always getting reset to actor final level
instead of engine final level (50, or 99).

This patch resizes parameters to the engine final level, but only
if they are smaller.
@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Sep 30, 2018

I found a bug in how we read RPG::Parameter when actor.final_level is not default.

With this fix I've tested:

  • hh3 v2.03
  • hh3 v2.04
  • Alter AILA Genesis
  • Dungeoneer

@Ghabry That test setup is awesome. Could you run your test suite again and tell me a couple sample games that still fail?

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch 4 times, most recently from e8ddb15 to c90f0c1 Oct 11, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 11, 2018

I've rebased this onto master. Also fixed the excel related differences.

This one is a huge pain to rebase. Basically have to redo fields.csv manually each time.

I verified rebased fields.csv against the old version using vimdiff, but this needs a full retest to make sure I didn't break it.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch 3 times, most recently from 271ed25 to 260774b Oct 12, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch from 260774b to 333bdb0 Oct 13, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch 4 times, most recently from 3d86e72 to 28e7989 Oct 17, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:ldb_enums2 branch from 28e7989 to 5361ab4 Oct 27, 2018

@fdelapena fdelapena removed the Needs Rebase label Oct 27, 2018

@carstene1ns carstene1ns force-pushed the fmatthew5876:ldb_enums2 branch from 5361ab4 to f7d2da8 Oct 27, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 27, 2018

codewise it looks fine to me but I will do a retest with LDB and also add LMT and LMU files. (maybe not all LMU files though because there are so many...)

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 28, 2018

Going to bed now, therefore only dumping you the raw output of my improved test script: (omitted LSD as this is part of another PR) and havn't filtered the bogus games you already reported...

For Maps I only used 0001 because I noticed that when the 1st map fails usually all other maps fail, too and using all maps far too long.

http://ix.io/1qkp

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 29, 2018

I don't see any new regressions for LDB from your list. All the ones there have some corruption or have terms issues.

Sweet Dreams (Demo) all the files (LDB,LMT,LMU) have 9 or 10 extra zero bytes at the end. I don't know what these are supposed to be for but liblcf doesn't write them back out.

This is the only one failing LMT testing, so we can sign off on LMT being done.

For LMU, are you sure your test is correct? I tested some of the failing games in your list and they came out clean, all LMU files.

Do you load the database and the treemap into memory before loading and saving maps? I do that in my test as they can rely on those data structures to parse correctly.

I'm using a hacked up version of the below test driver to do maps. The call to loadRPG() in there loads the LDB and LMT.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 29, 2018

My C++ test program is a bit less sophisticated but I'm indeed not loading the LDB or LMT which means the version field is not initialized... my fault. Won't have access to my PC before the evening, then I rerun it. I wonder if it will fix the 2k games, too (and then I wonder why).

I confirm that there are no new regressions which means I will remove all the non-fixable games from my test data.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 29, 2018

hm I messed something up, now I get for LMU

Summary:
2k:   1072/1092 (98.2%)
2k3:  402/416 (96.6%)
2k3e: 44/45 (97.8%)

but for LDB

2k:   14/1092 (1.3%)
2k3:  404/416 (97.1%)
2k3e: 45/46 (97.8%)

investigating...

@Ghabry Ghabry merged commit 448d1e2 into EasyRPG:master Oct 29, 2018

5 checks passed

GCW0 Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows Build finished.
Details
web Build finished.
Details
@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Oct 29, 2018

Found another flaw in my test code but fixing this requires going through all ZIPs again, but I had rates of 95% now, giving the real values when I finished fixing this.

@fmatthew5876 fmatthew5876 deleted the fmatthew5876:ldb_enums2 branch Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.