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.

Sign up for GitHub

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

Jump to bottom

/ ! \ data files from v1.08 up to v1.10 are faulty / ! \ #15

Closed
dlfrsilver opened this issue Sep 12, 2021 · 118 comments
Closed

/ ! \ data files from v1.08 up to v1.10 are faulty / ! \ #15

dlfrsilver opened this issue Sep 12, 2021 · 118 comments

Comments

@dlfrsilver
Copy link

Pyrdacor, i made different tests, and i have found where the problem comes from.

Basically, v1.07 from Thalion webshrine works fine on A500 512 chip +512 fast.

Version v1.08, v1.09, v1.10 just fail on A500 configuration.

I did a simple test : load the program from disk with disk A v1.07, then use disks B to I from the v1.10 set containing your data files modified.

It crash no matter what. So, there are 2 news, one good and one bad.

The good news is that all the modifications you did to fix the game data files are OK, no problem there.

The bad news is that you changed the data files byte structure (at least text data files like xMap_Texts.amb for instance).

How did i noticed that ? Simple :

I use the automatic Ambermoon data files ressourcer that CFOU, my partner in crime programmed for me in 2013 when i did the translation in french of the game.

The data files are crippled, because the ressourcer once i process the files from files 'xMap_Texts.amb' for example, outputs all the texts with the first character and the last character chopped off.

The problem never happen with v1.07 data files.

The game parser is very unforgiving on that matter. I seem to remember that you shorten some bytes in the text data files, but the parser seems to not like it......

What should we do now ?
@Pyrdacor
Copy link
Owner

Did you try the 1.10 version with 1.07 text files? If this is really the problem, it should work then.

@Pyrdacor
Copy link
Owner

But I wonder if this is really the cause. Emulating with WinUAE works fine. I don't think the text files are treated differently in emulation and real Amiga.

@dlfrsilver
Copy link
Author

What i mean is that you remove some bytes you thought were useless, but used by the parser.

As i explained, since you did not made any serious changes to the program code, and from my own discovery, my data file ressourcer should ressource data files from v1.08 to v1.10 without errors (it works with v1.07 data files).

When i ressourced your data files, the first character of a text bank and the last one are always cut off. This means that the data file structure has been changed or modified.

And the proof is in the pudding : I tested v1.10 program with v1.07 text files, and..... it works !

My ressourcer can't be faulty, because the programmer is one of the best on amiga, and he spent quite a bit of time to produce this tool, and I tested extensively the tool because it was my tool helper for translating.

The ressourcer takes decompressed amb extracted files, and then put them back as *.asm files. Then i have also the assembler that will re-assemble the ASM file to data file. My french version works on A500 and A1200 without crashing.

I suggest to stop the patching for the moment. we must ensure that the data files generated are correct. Otherwise we will go towards bigger problems later.

@Pyrdacor
Copy link
Owner

Pyrdacor commented Sep 12, 2021 •
edited

Maybe it's about word boundaries? Often load routines might read such data as words or dwords for speed. Can you give me an example which texts are crippled? Are there texts that are not crippled and if so do they have a length of a multiple of 2 or 4?

@dlfrsilver
Copy link
Author

here is what a ressourced data file looks like with my ressourcer :

BOPT	uo+
BOPT	ue-
BOPT	ua+

OUTPUT	0B6.amb

;Source Created by "Ambermoon Langage File ReSourcer" V1  
;A Tool done by CFou! on June 2013

Start
dc.w (BankListEnd-BankList)/2
BankList
_B00 dc.w Bank00End-Bank00+1 ;$004D
BankListEnd

Bank00 dc.b $20,"VOUS POUVEZ LIRE SUR L'ECRITEAU DE PIERRE:^^ ",$22,"LABYRINTHE DES GNOMES DE L'ILE DE MERA",$22,"",$20,0
Bank00End
dc.b $20
END

now compare with the same file, but generated from your versions data files :

BOPT	uo+
BOPT	ue-
BOPT	ua+

OUTPUT	0B6.amb

;Source Created by "Ambermoon Langage File ReSourcer" V1  
;A Tool done by CFou! on June 2013

Start
dc.w (BankListEnd-BankList)/2
BankList
_B00 dc.w Bank00End-Bank00+1 ;$004D
BankListEnd

Bank00 dc.b $20,"N THE STONE SIGN YOU CAN READ:^^ ",$22,"LABYRINTH OF THE GNOMES OF MERA'S ISLAND",$22,",$20,0
Bank00End
dc.b $20
END

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 12, 2021 •
edited

It's not about word boundaries. It's about text structures that are broken !

All the xMap_texts.amb file internal data files are crippled, all of them !

Look :

image

each text is starting by the byte '20'. This byte is missing from each data files made by your tools. This is part of the data files structure.

And the ending text banks bytes are missing too : the ending byte is not '00', but '20 00 20 00'

The parser is lost......

@Pyrdacor
Copy link
Owner

Can you try with the files from here? https://github.com/Pyrdacor/Ambermoon/tree/master/Disks/Bugfixing/English/Amberfiles

I just adjusted the text importer and generated the text files again.

@dlfrsilver
Copy link
Author

sorry, exactly the same problem, the missing bytes are still missing.

The error possibly comes from either the LOB compressor or your inserter/extractor tool.

@a1exh
Copy link

a1exh commented Sep 12, 2021

The original v1.07 xMap_,text.amb sub files regularly contain bytes at the end of them which are not described by the header. It has been proved previously these bytes are not used by the Amiga text parser because they have been removed by patches made by myself and Meynaf.

Can you provide a save game or a step by step description on how to trigger a crash.

@Pyrdacor
Copy link
Owner

I already fixed it I guess but can't push to github at the moment :/

@Pyrdacor
Copy link
Owner

Ok now the files are uploaded. @dlfrsilver can you now please test with the files. Sorry for the trouble.

@dlfrsilver
Copy link
Author

ok, let's me pick the files and test.

@alexh : there is no save to provide. Please look either above or on EAB forum what i did exactly.

The version A500 crash with v1.08 - 1.10 data files. the only thing that has really changed is the text data file structure.

The game crash just before displaying the text when you are in front of the the grand father.

@dlfrsilver
Copy link
Author

@Pyrdacor : the problem is now fixed.

Next step is to make ADF of v1.10 again to test.

@Pyrdacor
Copy link
Owner

I won't have much time for the next few days. But then I'll do so.

@Pyrdacor Pyrdacor mentioned this issue Sep 12, 2021
Closed
@dlfrsilver
Copy link
Author

Pyrdacor, i need to have all data files reviewed. not just the texts files. It still crash from ADF for the A500 version.

@Pyrdacor
Copy link
Owner

I never provided ADFs for 1.10. I thought it worked with 1.10 plus text files from 1.07.

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 12, 2021 via email

@Pyrdacor
Copy link
Owner

But it works with the text files from 1.07 you said so why should we review the other files?

@dlfrsilver
Copy link
Author

because you fixed only some files (texts ones).

As i said previously Robert, the current french version i have doesn't crash on A500.

I did also a try with v1.10 version under whdload (excellent tool to find if there is any problem on a game).

v1.10 crash whdload with an illegal instruction.

@Pyrdacor
Copy link
Owner

Pyrdacor commented Sep 12, 2021 •
edited

I guess we have to improve our communication. I thought you tested my 1.10 with only the text files from 1.07 and it worked.

I didn't change other data structures afaik. And I still wonder how data changes can influence crashes on the A500 while everything works on the A1200. I mean they won't have two implementations for different systems. Or is it related to AM2_CPU vs AM2_BLIT? Then maybe we should look for an issue there. I am not sure if something was changed there before my time.

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 12, 2021 via email

@Pyrdacor
Copy link
Owner

Can you try to use 1.10, copy only all the text files (including the dictionary file) from 1.07 over?

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 13, 2021 via email

@Pyrdacor
Copy link
Owner

Pyrdacor commented Sep 13, 2021 •
edited

I think the best approach is something like this:

Take the 1.07 version which works. Then add 1 file after the other from the 1.10 patch (https://github.com/Pyrdacor/Ambermoon/blob/master/Disks/Patches/PyrdacorFixEnglish1.10.lha). This contains every changed file. When it crashes after adding a file, you know that this file causes trouble.

You might want to skip the text files which I fixed in bugfix folder first.

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 13, 2021 via email

@dlfrsilver
Copy link
Author

Ok, I have done a simple test :

I took v1.07, and simply replace the file 1Map_texts.amb v1.07 by your version v1.10. This was enough to make the game crash.

@a1exh
Copy link

a1exh commented Sep 13, 2021

I took v1.07, and simply replace the file 1Map_texts.amb v1.07 by your version v1.10. This was enough to make the game crash.

What was your A500 setup? Are you playing from floppy disk? Hard disk? Kickstart version? RAM?

It crashes in the same place? When you start a new adventure on the first screen when it is displaying the text from Grandfather?

@Pyrdacor
Copy link
Owner

Ok, I have done a simple test :

I took v1.07, and simply replace the file 1Map_texts.amb v1.07 by your version v1.10. This was enough to make the game crash.

Why you would do this? Those are already fixed. Take the fixed file and progress with the others which I didn't recently fixed.

@dlfrsilver
Copy link
Author

Why would i do that ? Simply to test and point out that the game crash.

What i did is simple :

  1. copy the file 1Map_texts.amb from v1.10 on disk D (version v1.07) as you request above.

  2. load the game with A500 configuration and 2 drives (the game is a nightmare to load with 1 drive).

  3. the game will ask you disk D and G alternatively, and then once loading has finished, the game will crash.

@dlfrsilver
Copy link
Author

here is visually what a correctly crunched LOB packed file look like :

image

@nicodex
Copy link

nicodex commented Sep 15, 2021 •
edited

here is visually what a correctly crunched LOB packed file look like :

Doesn't matter, I developed a compression function that haven't been beaten, yet. This reduces memory, copy, and extraction overhead. For the copy operation it would be even better to longword align the streams (MC6020+), or to not compress some streams at all.

@Pyrdacor
Copy link
Owner

@dlfrsilver Nico already decoded everything in detail and he is skilled in m68k. So we can count on what he found. Please read his link carefully.

I will adjust my packer to take care of it tomorrow and then we can test again.

@nicodex Could you provide the additional buffer sizes or the offset in the executables? That would be awesome.

@dlfrsilver
Copy link
Author

next look at this compare : On the left, the file 1map_texts.amb correctly encoded with InsertAMB after inserting the files crunched with MSS encoding with LOB packer v3.7 on the left, and on the right, the same archive encoded with AmbermoonPack.exe with missing 'LOB' header :

image

@dlfrsilver
Copy link
Author

Doesn't matter, I developed a compression function that haven't been beaten, yet. This reduces memory, copy, and extraction overhead. For the copy operation it would be even better to longword align the streams (MC6020+), or to not compress some streams at all.

The goal is to make the game able to run from floppies, like the original. And basically, your enhancements are fine for a windows PC version, but here i'm writing about the Amiga version running on original hardware.

I know that Ambermoon is very picky on modifications. So i will ensure that the game works like originally intended.

@nicodex
Copy link

nicodex commented Sep 15, 2021

@nicodex Could you provide the additional buffer sizes or the offset in the executables? That would be awesome.

CmpSpace in this table: https://gitlab.com/ambermoon/research/-/wikis/files#file-ids (0 = not compressible)

@Pyrdacor
Copy link
Owner

@dlfrsilver Do you even read what I write? The compressed output doesn't have to be equal. Every compressor can work and compress differently and still it can be valid compressed data.

@kermitfrog
Copy link

Had to have a look at this thread, as my email folder was flooding..

I don't have much time to help right now (maybe tomorrow; more likely saturday - will join if you haven't solved it by then).
The alignment issue seems a good canditate for the cause, but I have one more (less likely) idea: could it be the kickstart version? Maybe the LOB compressor uses some system call from there, that is buggy in some versions?

@nicodex
Copy link

nicodex commented Sep 15, 2021 •
edited

The goal is to make the game able to run from floppies, like the original.

With a better compression routine you could even put more data on the floppies...

https://gitlab.com/ambermoon/research/-/blob/master/src/am2crunch.c#L235
(I recommend to also read all the comments in am2crunch.h - cmpSpace-check and longword-alignment is not yet implemented, got hooked by other things)

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 15, 2021 via email

@Pyrdacor
Copy link
Owner

true Nico, but this is another subject :) It goes beyond our need :) Le mer. 15 sept. 2021 à 20:06, Nico Bendlin @.***> a écrit :
…
The goal is to make the game able to run from floppies, like the original. With a better compression routine you could even put more data on the floppies... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVPMYYX5MC7BUKUG6HMKLT3UCDOBPANCNFSM5D4LBNNA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Please be patient and let me adjust the packer tomorrow. Then we will see if it works, ok?

@Pyrdacor
Copy link
Owner

@nicodex Could you provide the additional buffer sizes or the offset in the executables? That would be awesome.

CmpSpace in this table: https://gitlab.com/ambermoon/research/-/wikis/files#file-ids (0 = not compressible)

OK so all savegame related files are not compressable.

Would be interesting to see your approach for finding longest matches. Did you see my implementation yet? Not sure how good it is, but it compresses a bit better than the original. But I guess the word alignment might reduce effectiveness.

@dlfrsilver
Copy link
Author

Pyrdacor, what is the command if i want to unpack an archive with with hexa filename instead of numbers ?

example : '016' in hex instead of '022' ?

@dlfrsilver
Copy link
Author

Please be patient and let me adjust the packer tomorrow. Then we will see if it works, ok?

Ok, i will then toy with the french version :)

@Pyrdacor
Copy link
Owner

Pyrdacor, what is the command if i want to unpack an archive with with hexa filename instead of numbers ?

example : '016' in hex instead of '022' ?

Add -x like AmbermoonPack.exe UNPACK 2Map_data.amb 2Map_data -x

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 15, 2021 via email

@dlfrsilver
Copy link
Author

Hello, please try also with this file (i have refactored it with the LOB packer, with the help of the excel file from Pyrdacor data fixes) :

https://we.tl/t-SNPij8OC8Z

@Pyrdacor
Copy link
Owner

Hi @prophesore. Sounds like some text is referenced which is no longer present. I assume the "nothing" entry. Between 740 and 014 there is the last and first row of world maps. Does it happen at every x location or only in a similar x range (740 to 040)?

There was a reported bug that you can't reach location 0,0 but I could not reproduce it. Maybe it is related?

I will have a look at the snakesign crash as well.

@Pyrdacor
Copy link
Owner

I had a look about the gemstone coordinates. It might all related to the snakesign map texts.

@nicodex
Copy link

nicodex commented Sep 15, 2021

[...] but I have one more (less likely) idea: could it be the kickstart version?

Once I found an issue in the v1.04 patch with v39+ Kickstarts. Of course the game checks the version, but on closing the game it passes random data to a lib call. Not related (because it is cleanup code), but might still be there in v1.05.

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 15, 2021 via email

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 15, 2021 via email

@dlfrsilver
Copy link
Author

Done : here is the v1.10 in ADF, with all the files that needed rework done :

https://we.tl/t-2OAXPwKhkO

Enjoy !

@dlfrsilver
Copy link
Author

it should not happen for very small files like 1Map_texts.amb. Did you really test to only replace this single file?

Are you are asking me? No. I replaced 2Map_texts.amb that contains the data at the very start of a new game.
I can try JUST 1Map_texts.amb if you give me an example of where in the game this data is used.

file numbers in 1map_texts.amb

0-nothing (6 bytes)
58-oasis water of life 525/155
87-before Illien
106-in front of the crypt next to Nelvin's Tower
125-Temple of Gala-5 texts
138-Orc Cave-Selena + in front of Old Dwarf Labirynth
140-hut on the Hoimon Desert-3 texts
148-Fire Thistle
154-Orc Cave-Weird Stone-2 texts
155-lizards in the Hoimon Desert
156-signpost in front of the Hoimon Desert-2 texts
181-before the Labirynth of Gnomes with Targor
184-Lebab Tower-4 texts
185-Kalmir Herb-2 texts
197-temple of the winds-2 texts
229-rock against the Donners Old Labirynth-2 texts
232-signpost at Snakesign-2 texts

I play E-UAE on A1200 Blizzard 30/50 v1.09 and v1.10.
The first island is easy, but you can't get to the Gemstone after that.
After reaching line 740 from the North and 014 from the South, the game shows an error in the map file.
You can fly to the Gemstone from the northwest but getting to the wall is a crash.
Pass through the teleport to the Snakesign crash. I stole an eagle and landed inside a Snakesign but crash upon entering the gate.

After losing 1map_texts.amb from version 1.07, everything is fixed. Maybe the problem is cooperation with 1map_data.amb?

To Pyrdacor - in the file with errors in Excel, he writes that the attributes of Valdyn and Leonaria have been corrected - not true (maybe in version 1.07e or WHDLOAD?).

@prophesore : from what you say, it looks like you play with an unpatched version. What you point has been fixed.

@nicodex
Copy link

nicodex commented Sep 16, 2021 •
edited

Would be interesting to see your approach for finding longest matches.

I'm using a topologically sorted weighted directed acyclic graph. Ultimately, it boils down to generating all possible code variants and finding the shortest path (in coded bits) from the beginning to the end. The temporary memory requirement for the graph is (uncompressed_size + 1) * 4 * sizeof(size_t), which is expected to be available on every build system (the Ambermoon data blocks are not that large).

https://gitlab.com/ambermoon/research/-/blob/master/src/am2crunch.c#L244

I also started to add optimizations for the encoding of the alignment, but the code is neither ready, nor published (not that important, because it might only save a single byte in corner cases, which might be eaten by the alignment requirements).

Did you see my implementation yet?

No spare time for any review yet, sorry :)

@a1exh
Copy link

a1exh commented Sep 16, 2021

Out of curiosity. Were the changes @Pyrdacor made to the TextWriter necessary? They were not the cause of the crash. Not suggesting reversing the change but I am curious to know if the changes were beneficial or superficial.

@Pyrdacor
Copy link
Owner

@a1exh Good question. I don't think it was necessary but I can't tell for sure. If you have some minutes you could test yourself by taking 2Map_texts.amb from 1.09 english (unfixed LOB and trimmed texts), depack it and try to use it on a 68000 CPU.

@nicodex Interesting. I used a Patricia Trie but I know it's not the best approach. I think sequences of identical bytes could be compressed much better. I will add this as an optimization later. Especially when thinking about Ambermoon Advanced and much data to add. :)

Thanks again for your insight. I guess you could help us a lot to understand some parts of the executables. Maybe even fix some code-related bugs.

The LOB compression works fine now and I released updates for 1.10 english and 1.09 german. Moreover I added the ADF images.

@dlfrsilver
Copy link
Author

dlfrsilver commented Sep 16, 2021 via email

@a1exh a1exh mentioned this issue Jul 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@kermitfrog @nicodex @Pyrdacor @a1exh @dlfrsilver and others