[Project] Ambermoon bugfix patches

[Pyrdacor]

The goal is to create a stable patched Ambermoon version in german and english. We focus on the english patch for now.

[Pyrdacor]

I decided to first add writers for all game objects like monsters, NPCs, texts, etc. I partially need this anyway to save Ambermoon.net games back to original savegames.

With those writers and the existing readers you can then do the following in code very easily:

• Load a monster
• Change a monster value
• Save the monster

Combined with a decompressor/compressor and archive loader/writer this should allow for easy editing whole game files like "Monster_char_data.amb".

The archive loader and writer functionality is already there with decompression and compression support.

So when I finish the game object writers (which isn't too hard), the game data patching should be very easy.

[Pyrdacor]

The implementation of the writers will be tracked here: #53

[Pyrdacor]

@a1exh I will keep you up to date regarding data writer implementation status. Afterwards I'll need your support to fix those bugs and create a patch.

[a1exh]

Sounds good. I'll try to make some diff files for the more obscure bug fixes to help satisfy my curiosity

[Pyrdacor]

@a1exh The character writers are ready (monsters, NPCs, party members) and the text writers too. So we could start fixing. Items will be much harder though, but maybe the first patch version could just fix characters and texts. Map writers are missing too but map texts can be fixed already.

[a1exh]

Object_texts.amb sub-file 001.amb this file has a bug in it where it uses the word "GRANDSON" regardless of the gender of the main character. In my bugfix I change the text to "GRANDCHILD". I think I didn't know if there was a scripting special markup gender specific code e.g. ~ SEX1 ~ (HE/SHE) AND ~ SEX2 ~ (HIS/HER) for things like this. But I don't think there anything as specific as SON/DAUGHTER

[a1exh]

v1.07 has an issue where if you're flying/sailing near the coordinates [0,0] (might be [1,1]) that there is a reference to a missing text block in 1Map_texts.amb that causes the Amiga version to crash. This was patched in two different ways, Metallic fixed it by adding a dummy text block 001.amb into 1Map_texts.amb so that it didn't crash. Meynaf said he removed the reference to the missing text block but he didn't say where or how he did this. The Meynaf patch doesn't modify 1Map_data.amb. I'd be curious to know what the underlaying cause of this is and how Meynaf patch works.

[a1exh]

I've been working for years on the list of bugfix patches for v1.07

https://docs.google.com/spreadsheets/d/1as5W8gibm-MTb9VEqpkfgtwWviqjQx96A3NmcvzX98A/edit?usp=sharing

The "sub-filename" number is the one produced using the Amiga tool by Metallic "ExtractAmb" to split files into their components.

Most bugs have two sub-files to DIFF to see the changes (however the files sometimes contain fixes for more than one bug)

You can sort the spreadsheet using the filename column to put them into file order.

I'm going through on a file-by-file basis starting with v1.07, Party_char.amb.

If I am going too slow for you then for v1.07 take a look at the modified version by Meynaf

http://meynaf.free.fr/pr/amberpatch.lzx

The patch is not comprehensive (there are known fixable bugs he didn't fix and at least one with a mistake) and it contains fixes not detailed in the list of fixes in the text file included in the archive.

And for v1.05 take a look at the modified versions by Daniel Schulz

http://slothsoft.net/getResource.php/amber/patch/Ambermoon.v1.06.zip

I don't know much about v1.05 as I don't read German but I believe he's fixed most (all?) of the common bugs between v1.07 and v1.05. He has a list v1.05 bugs albeit not down to the specific bytes :

http://slothsoft.net/Ambermoon/EditorHelp/ (look at the section : MIR BEKANNTE BUGS IN AMBERMOON)

[a1exh]

If you don't know in which file a bug may be located, feel free to ask. I am very familiar to the internal data structures and their relations already.

Where are the real opening times of all the shops stored? Lots of them have the wrong opening times in the v1.07 text blocks. Most have patches where people have worked out "in-game" the real opening times, but one or two remain. For example 2Map_texts.amb sub-file 119.amb ALCHEMISTS TOWER - "ALCHEMISTIC ACCESSORIES^ OPEN FROM 8 AM TO 8 PM" but I have bug reports it wasn't open when they visited at 18:45

[Pyrdacor]

Object_texts.amb sub-file 001.amb this file has a bug in it where it uses the word "GRANDSON" regardless of the gender of the main character. In my bugfix I change the text to "GRANDCHILD". I think I didn't know if there was a scripting special markup gender specific code e.g. ~ SEX1 ~ (HE/SHE) AND ~ SEX2 ~ (HIS/HER) for things like this. But I don't think there anything as specific as SON/DAUGHTER

No there is only he/she and his/her. So I guess grandchild is a good solution.

[Pyrdacor]

v1.07 has an issue where if you're flying/sailing near the coordinates [0,0] (might be [1,1]) that there is a reference to a missing text block in 1Map_texts.amb that causes the Amiga version to crash. This was patched in two different ways, Metallic fixed it by adding a dummy text block 001.amb into 1Map_texts.amb so that it didn't crash. Meynaf said he removed the reference to the missing text block but he didn't say where or how he did this. The Meynaf patch doesn't modify 1Map_data.amb. I'd be curious to know what the underlaying cause of this is and how Meynaf patch works.

In general text popups are triggered by specific map events. They are stored inside the map data. It is possible to fix the text index or remove the event. But if Meynaf didn't change the map data file this is strange. Another possibility are NPCs. They also can use map texts in some cases. But I don't think there is an NPC on that map.

[Pyrdacor]

If you don't know in which file a bug may be located, feel free to ask. I am very familiar to the internal data structures and their relations already.

Where are the real opening times of all the shops stored? Lots of them have the wrong opening times in the v1.07 text blocks. Most have patches where people have worked out "in-game" the real opening times, but one or two remain. For example 2Map_texts.amb sub-file 119.amb ALCHEMISTS TOWER - "ALCHEMISTIC ACCESSORIES^ OPEN FROM 8 AM TO 8 PM" but I have bug reports it wasn't open when they visited at 18:45

The opening times are stored in the map events of each map. Only hours are stored. There are two places in Alchemists Tower: a merchant (sells goods) and a library (sells spell scrolls). In my current data the following open times are set for them:

• Merchant: 8-18 (8am to 6pm)
• Library: 8-20 (8am to 8pm)

[a1exh]

Thanks. I can change the text and move this bug to the FIXED page.

[Pyrdacor]

@a1exh I don't know if this bug was reported before but I found something during battle testing in original:

If you choose to attack a nearby monster with a close-ranged weapon (e.g. Scimitar) and then open the inventory and exchange this weapon with a long-ranged weapon (e.g. Crossbow) you will be able to attack with the long-ranged weapon without consuming ammunition. Moreover the symbol for close-ranged attack is displayed near the portrait.

[a1exh]

Thanks this has never been reported. Is it 100% reproducible?

[a1exh]

Daniel Schulz has noted two semi-related things

1. Magic Bolts require 0 hands and so can be combined with a 2-handed weapon to give enchantment bonus of attack +15

2. Sling Knife and Sling Dagger have an attack value even though they are ammunition and therefore you should equip rather than a shield

It is not obvious if these are bugs or as intended.

[Pyrdacor]

Thanks this has never been reported. Is it 100% reproducible?

I only tested with german v1.01 (22-11-1993). But with this version it is reproducible.

[Pyrdacor]

@a1exh Found another minor bug. The spell Blink is cast on a friend to teleport him in battle. But if the friend moves before the spell is cast, a message like " has been blinked" is shown. So there is no character name. I guess the game looks at the spot for a character but does not find one anymore. The character is not blinked of course.

I would expect one of the following:

• Blink the moved character
• Let the spell fail and show a message about it

I decided to implement the "fail message" in Ambermoon.net. It is a bit odd but in Ambermoon even support spells like Healing can miss if the character has moved before casting. So I added this behavior to Blink as well to be consistent. It is kind of a game mechanic. You have to consider speed when casting supportive spells and maybe have to increase the Speed attribute of supportive members like Healers and Alchemists.

[Pyrdacor]

@a1exh I wonder if someone already reported this but you can create a character with an empty name. In this case no name is shown in inventory, at the top portrait and in conversations.

[a1exh]

No one has mentioned it. But it is not really a bug unless it causes crashes due to string length errors etc? No?

[Pyrdacor]

True. I don't know if this can cause problems in some situation but even if, this wouldn't be much of a problem as you can just pick a valid name. ;)

[kermitfrog]

Bug: when the monster ai is determining which spell to cast and on which target, there is a bug in the row spell targeting:

        0023c23a 72 00           moveq      #0x0,D1
        0023c23c 74 00           moveq      #0x0,D2
        0023c23e 76 00           moveq      #0x0,D3
        0023c240 7e 18           moveq      #0x18,D7
                             LAB_0023c242                                    XREF[1]:     0023c258(j)  
        0023c242 0f 00           btst.l     D7,D0
        0023c244 67 0a           beq.b      LAB_0023c250
        0023c246 26 52           movea.l    (A2)=>battlefield.fields[24],A3                  = null
        0023c248 36 2b 00 22     move.w     (0x22,A3),D3w
        0023c24c d2 83           add.l      D3,D1
        0023c24e 52 42           addq.w     #0x1,D2w
                             LAB_0023c250                                    XREF[1]:     0023c244(j)  
        0023c250 58 8a           addq.l     #0x4,A2
        0023c252 52 47           addq.w     #0x1,D7w
        0023c254 be 7c 00 18     cmp.w      #0x18,D7w
        0023c258 6b e8           bmi.b      LAB_0023c242
        0023c25a 4a 42           tst.w      D2w
        0023c25c 67 02           beq.b      LAB_0023c260
        0023c25e 82 c2           divu.w     D2w,D1

The loop ends when D7 is > 0x18, but it is set to that value at the loop's start and then only incremented, so the loop will always exit after 1 pass. 0x18 is 24, the index of the first field in the lowest row on the battlefield. The loop calculates the avarage last damage and is probably copied from a one before it, which does the same for the row above it. Then they forgot to change the exit condition...

I think changing

0023c254 be 7c 00 18     cmp.w      #0x18,D7w

to

0023c254 be 7c 00 1e     cmp.w      #0x1e,D7w

should be enough to fix it.

[Thallyrion]

Even though this might technically be a bug i would hesitate fixing this in the original Amiga Version as it could change the Gameplay significantly.

[Pyrdacor]

At least we should add it to a bug list so that it is documented. Maybe someone wants to release some advanced version later with better balancing and fixed mechanics. Some things in Ambermoon feel quiet buggy or out of balance. Some of those might just be small bugs in the code like this one.

[Pyrdacor]

@a1exh Found another bug in original. When you buy items you store them in a temporary item grid. This grid has 4x6 slots (same amount as the merchant). But the merchant can stack all items. For example the merchant in Spannenberg sells 15 sandals as one stack. Sandals can't be stacked otherwise (inventory, chests, etc). This is also true for the temporary item grid. Sandals can't be stacked there.

This way you can fill the temporary item grid with more than the 24 items. And when you do so the game stops to react to any input. The only thing that still works is the mouse cursor.

So to reproduce the bug go to the merchant in Spannenberg and buy more than 24 non-stackable items (e.g. Robe, Leather shoes and sandals). Then the game should stop reacting.

A possible fix would be to steps:

• Disable the "buy" button when the temporary item grid already has 24 items
• Limit the amount you can choose from the "amount input box" for unstackable items to the amount of free slots in the temporary item grid

This is also the way I implement it in Ambermoon.net.

[Thallyrion]

Another bug or feature? is about the elven harp. When you give the 7 crystal strings to Matthias the harper you get the crystal harp even without having bought the elven harp before. I guess this is a bug as the conversation states that he takes the strings AND the elven harp from you to make the crytal harp.

[Pyrdacor]

This can be fixed easily inside the NPC data by adjusting the conversation events.

[Pyrdacor]

I also think this is a bug. @a1exh should add this to the bug list. I can analyze the bugged data and provide a fix instruction.

[a1exh]

Thanks. I've added it to the bug database. The location is ILLIEN, HOUSE OF THE HARPER. The associated text is NPC_texts.amb sub-file 01D.amb

Please let me know if this affects both v1.07(Eng) and V1.01(Ger) and the exact details on the fix when you have them.

[a1exh]

A save-game with the 7 crystal harp strings nearby would be good for testing if anyone has one.

[Pyrdacor]

I will take the strings from the Thalion office and fly there with an eagle to test it. But I guess I can already see the cause inside the NPC data.

[Pyrdacor]

In the NPC data only the existence of the strings is checked. Not the harp. There is no condition event for it. To provide a fix I will need some more time. Adding the condition event is easy but maintaining the correct linked event list is a bit more tricky. Will come back to it soon.

[Pyrdacor]

I found another very serious bug in Ambermoon. If you cast "Repair item" on any item and it fails due to bad "Use magic" ability the item breaks. The problem now is that this also happens if the item isn't broken and even if the item is important.

For example you can destroy the Flute with it. You can destroy all items this way and break the game.

The problem here is that the check if the item is broken should come before the "spell success" check.

Edit: This is also true for "Charge item" and "Duplicate item".

[Pyrdacor]

Another very minor issue. In Newlake Inn you wake up on a table. :D Every Inn teleports you to the bedroom while resting. This is represented by a map index and the position (x, y). Both stored inside Place_data.

The fix would be to change byte at offset 0x725 from 0x16 to 0x15 in Place_data. This way you will stand next to the table instead of on top of it. But maybe this was on purpose and you had a pleasureful night with loads of alcohol and really woke up on that table ...

[Pyrdacor]

Another possible bug. Has to be tested first but I'm too busy for it.

It seems that you can buy horses or boats as long as there is none at the target location. So if you buy a horse and move it away afterwards, you can buy another one. There are only 32 slots for horses, rafts and ships in the savegame. So I wonder what will happen if you buy more than 32 horses/ships. And if you buy too many horses, can't you buy the first Ship from Torle anymore?

@Thallyrion @kermitfrog @a1exh How should I implement this in the rework? Limit to just 1 Horse/Ship? Or limit to the 32 possible slots and leave room for at least 1 ship and 1 horse? Other ideas?

Can someone test the described bug please?

[a1exh]

A wrapping queue would be best. Overwrite the 1st entry with the 33rd entry

[Pyrdacor]

The problem is that these 32 entries do not only contain bought transports but also those that are initially placed (like the raft at the second island). So these ones should not be overwritten at all imo as it would break the game.

[Pyrdacor]

More precisely there are 3 initial transports in the savegame: 2 ships and 1 raft. Afaik there is only 1 horse seller and 1 ship seller and no raft seller. So in theory there are 29 slots for the use of bought horses and ships. As there are already 2 initial ships (1 in the main ocean), I would suggest to allow 15 total horses and 16 total ships (1 is inside the Twinlake water ring).

Another option would be to allow 10 of each of the 3 types (horse, raft, ship). This could be useful if the game is modded later (e.g. adding a raft seller -> the game has texts/images for it). Nobody will need more than 10 horses or ships I guess.

My favorite solution is to allow 12 ships, 10 rafts and 10 horses. This leaves room for game modifications, won't really affect the original game experience and I guess the ocean is the biggest part of the world map (ship), rafts can only be used around some isles and horses only on islands (in original on a single one).

[a1exh]

Whatever is easier. It would be good to prove this is an actual bug in the Amiga version and see what happens when you exceed 32 entries.

[Pyrdacor]

Agree. It takes some time to move to Tolimar, leave Spannenberg and so on for 29 times. Maybe someone can help me out.

If someone will do, please also provide a savegame with 32 slots filled so I can also test the implementation in the rework. Thanks.

[a1exh]

Is it not possible to edit the savegame? I don't have any free time for 2 weeks but I will set aside time for this and the other bug you asked for help with 13th&14th March.

[Pyrdacor]

Yeah should work. It isn't that urgent. I have used a temporary solution now. But I want to focus on getting things implemented. ;) Just finished implementing the 3 sellers for horses, ships and rafts. Will adjust the code when testing is done.

[Thallyrion]

I have testet the this with horses. Indeed after buying 29 horses you won't be able to buy anymore. I simply would leave this issue as it is. I can't think of any scenario where you would buy 29 horses or ships. I never bought more than one, so this should not be a real issue. You can't return to the ship merchant without your ship anyway.

[Pyrdacor]

Looks funny with all those horses. Ok at least the game won't crash and disallows further buying. I guess it's ok to leave the bug unfixed. But I will limit it a bit in the rework as I want to keep the opportunity open to extend the game later and maybe add some new NPCs etc.

Thanks for testing!

[kermitfrog]

Rarely occurring, minor bug in battle AI:

on a battlefield setup like this, with O = empty, M = monster, P = party member, x possible attack spot

OOOOOO
OOOOOO
OOOMMM
OMOxPO
OOOOOP

The leftmost monster could arrive at a position x in two turns, but will instead just sit in place.

This is caused somewhere in FUN_0023c5d0 or FUN_0023c674 (English version, Function names by Ghidra) and in my opinion very difficult to fix in original Ambermoon - so I'm posting this here for completeness.

See this comment for a little more info: https://github.com/Pyrdacor/Ambermoon.net/issues/64#issuecomment-791307337

[Pyrdacor]

@a1exh I started a small project which allows patching the game data files. The patcher takes a patch file which provides the information about the fixes. Fixes can use data replacements, insertions and deletions and can combine them. You can specify multiple fixes per patch file. Each patch file represents a game patch.

I wrote a small readme here: https://github.com/Pyrdacor/Ambermoon/blob/master/AmbermoonTools/AmbermoonPatcher/README.md

The project already works as far I can tell. I will release it soon.

It interprets a simple script language I created today.

Let me know what you think about it?

[Pyrdacor]

An example patch file could look like this:

# Fix 123: Gryban waiting location
- Replace Party_char.amb[15]:0x1C 'ff ff'

# Fix 124: Some other fix
- Delete 2Map_texts.amb[$3C]:$5 20 // Delete 20 bytes at offset 5 from map text subfile 0x3C
- Insert 2Map_data.amb[258]:1000 2Map_data[259]:1000,100 // Copy 100 bytes from map 259 to 258

I think you get the picture.

You can store this file as an example with the name "Ambermoon_Patch1.08.amp" and the patcher will apply all fixes to a given game version.

The lines starting with # mark single fixes and also are a good documentation about the content of a patch. Humans can easily read and understand those patch files I hope.

[Pyrdacor]

I will also add commands to replace, insert and delete texts and parts of texts.

The plan behind this is to provide patches in form of a script file or at least parts of a patch. This way you could patch a given version to a new version with several fixes. Moreover it will be easier to create new patches in the future and to track which fixes belong to a specific patch.

You can also generate some excel, HTML or markdown output from such patch files so that you get some kind of changelog.

I could also add more possibilities for meta data like "bug reporter", "version the patch should be applied to", "detailed bug description" and so on. I could mimic parts of your excel bug list with the script in a way you could produce the excel file from the script and also use the script to patch a game version.

[Pyrdacor]

Found another bug in Ambermoon. If you use "word of marking" to save your location on the lyramion world map, then call the eagle and cast "word of returning" from the eagle, you will have positioning bugs when moving around. Maybe someone can test and confirm this. Maybe this is also true for other transports like the witch's broom etc.

[Pyrdacor]

Next bug found: There is an award event in Ambermoon. It is used to grant Exp, LP, SP, Attribute increases and so on. It can increase (value or percentage), decrease (value or percentage), fill, remove, add and toggle (last 3 for ailments, languages and spell classes).

Actually there are 10 different affected stats (Attribute, Ability, LP, SP, SLP, Ailment, Usable spell types, Spoken languages, Exp and TP). But Ambermoon only handles the first 9. So the TP change events have no effect. But they are actually used. For example after giving the TARBOS amulet to Tar or giving Alkem's ring back. In addition to the Exp, there would normally be TP increases of rand(0 to 25) and rand(0 to 15) but they are not processed.