diasurgical / devilution

Diablo devolved - magic behind the 1996 computer game
Other
8.79k stars 919 forks source link

Document Diablo's bugs #64

Open ghost opened 6 years ago

ghost commented 6 years ago

Currently documented bugs: The DSF Buglist for Diablo v1.09 (Lurker Lounge)

The goal of this issue is to document any new bugs we come across while examining the code that are native to the original game. This will help us fix them later on when we make mods/ports.

New bugs discovered (last update 06/24/18)

fearedbliss commented 6 years ago

Are these bugs going to be fixed in devilution after devilution development becomes stable or is the devilution source code gonna be a pure documentation update including leaving in the vanilla bugs?

ghost commented 6 years ago

The Purpose section of the readme has the answer.

fearedbliss commented 6 years ago

Ah ok, I did read this like a week ago but I guess I forgot ;D.

ttdonovan commented 6 years ago

The pull request above is basically a copy of the text from the original posting with some Markup for formatting. Is something like this is what is being requested?

ghost commented 6 years ago

We can definitely make use of your copy in case the original one goes down. However, this issue is to track new bugs that haven't been documented anywhere else, so that we can fix them in the future.

(Not Devilution of course, but mods based on)

ghost commented 6 years ago

As a rule for any bug published can we have the save file for this? I would like to have something to make this more easy to replicate...

because I am thinking on looking at the TP sequence and the Chamber of bone locations... because Technically TP is working but it's not marking the chamber of bone as a different map , if I am not mistaken...

EDIT:

I am probably completely wrong on that one. The dungeon map array looks ok for the most part...

Perhaps the Portal setting and getting is wrong? Perhaps it's not specifying the correct map for special maps?

Ok . Which memory map (showing address locations) is the most accurate for the origional diablo? Do you have it ? All the addresses I am seeing appear to be inaccurate.

ghost commented 6 years ago

@ApertureSecurity check Support/surgery.xls for a spreadsheet containing addresses.

Found another one while testing the world.cpp refactor. This bug is also part of vanilla.

EDIT: This bug was Devilution only https://s33.postimg.cc/w9y0yf6ov/doorflip.png

ghost commented 6 years ago

Brevik himself talked about this bug in an interview once. The Caves were entirely hardcoded and many fixes added per-tile: cave bug

ghost commented 6 years ago

@galaxyhaxz is this in the origional game or are we going to add this to our bug list?

ghost commented 6 years ago

Well, it looks like the door issue mentioned above was devilution specific. However, these pieces are saved with the character so it becomes bugged when loading the character in a vanilla game. https://github.com/diasurgical/devilution/commit/ee5675108eb0b8e169b5f154ccc4dee8a11d4abe

mewmew commented 6 years ago

Well, it looks like the door issue mentioned above was devilution specific. However, these pieces are saved with the character so it becomes bugged when loading the character in a vanilla game. ee56751

Interesting find. This is good to know when validating bugs in the future against vanilla Diablo. Basically, we can use the same seed for dungeon generation when validation, but we can't use save files.

ghost commented 6 years ago

There is an overflow bug in the dungeon algorithm of Cathedral that sometimes causes out-of-map pieces. Documented here.

I can confirm that Devilution works correctly (i.e. is bugged like vanilla). Current known seeds:

Cathedral:
2588
4743
7281
9345
15236
ghost commented 6 years ago

Eldritch Shrine Bug

The Eldritch shrine (turns healing/mana pots into rejuvs and full pots into full rejuvs) uses the player's holding item buffer to temporarily store the rejuvs. What could possibly go wrong here?........ xD xD

If you click on the shrine and then pickup an item from your inventory/belt, the item you're holding will get overwritten. When you place it back down, it turns into a rejuv. BEWARE!

As a side note, this can be exploited for an extra rejuv, grabbing say 1 piece of gold or some junk item.

Bugfix

The routine for this in OperateShrine should use a temporary buffer local to the function or the global temporary buffer curruitem to fix this. :)

mewmew commented 6 years ago

As a side note, this can be exploited for an extra rejuv, grabbing say 1 piece of gold or some junk item.

Haha, great hack!

ghost commented 5 years ago

Edit: actually the spaces in yellow are spots where there aren't any pixels in the .CEL, so the game never draws anything there. The function responsible for drawing the void fills them black. The only way to fix this is to edit the CEL map and create pixels in the empty spots. DIABLO_2019031337_061641 DIABLO_20190317_221850

ghost commented 5 years ago

By chance I stumbled upon a broken tile on level 5. Here is the picture with seed and location: _seed_bug_lv5

mewmew commented 5 years ago

It's great. We are gonna be able to create a version of Diablo that fixes all known bugs. Of course, Devilution will contain all original bugs to stay true to its origin.

ghost commented 5 years ago

Another bug where standing in certain spots overwrites the top left corner of the control panel: _tile_bug_lv1

mewmew commented 5 years ago

Another bug where standing in certain spots overwrites the top left corner of the control panel:

Is this in vanilla too? Never seen the panel being incorrectly rendered before.

AJenbo commented 5 years ago

All your belt items also went invisible. Could be a miss calculated CelSkip/CelCap

ghost commented 5 years ago

@mewmew Yeah it's in vanilla. You won't notice the bug unless you hover your mouse over that spot in the control panel, it overwrites the panel graphic. I rewrote the whole render and discovered this bug while testing. It's likely caused by DrawMain not blitting all parts of the screen all the time.

@AJenbo The belt items is just a side effect of the new render, not in vanilla. If you notice there's transparency! <3

Edit: attached save file so you can test it. Load the game and place mouse over top left corner. single_4.zip

Interesting is that this bug doesn't happen in the debug release or prior. So it was caused by something changed in the render in 1.00.

ghost commented 5 years ago

@AJenbo had pointed this bug out awhile back, as it was thought to be caused by my render re-implementation. Actually, it's caused by the whole eflag system where for some reason, when walking south east, arches aren't drawn above the player. So the fix was to add separate drawing code outside of the render. I had to catch it mid action, but the spots in yellow are where eflag tiles are drawn. They overwrite the book case. IIRC this bug was fixed in the PSX version, since they ditched eflag entirely. DIABLO_20190428_063210

AJenbo commented 5 years ago

Nice catch, my print screen has a delay so getting shots like this is a pain :)

The other yellow box is causing issues when we upgrade the render to have per-pixel transparency because it renders the tile twice, resulting in 75% opaqueness instead of 50%

AJenbo commented 5 years ago

Here's another render issue: Caves

https://github.com/diasurgical/devilutionX/issues/111

qndel commented 5 years ago

In the very tristram itself :) image

Zoomed: image

I'ts more obvious in tchernobog, that's what made me notice it :P

ghost commented 5 years ago

@AJenbo I believe that issue is the same one I reported above. There's a broken tile that can occasionally spawn in the caves that does that.

Here is a list of seeds for Caves with that broken tile and the coordinate of it:

seed 24, x 28, y 10
seed 206, x 19, y 27
seed 265, x 27, y 6
seed 534, x 26, y 20
seed 1714, x 24, y 25
seed 1980, x 12, y 14

Here is code to fix the tile by brute force. It should be ran during DRLG_L3:

    for(j = 0; j < 40; j++)
    {
        for(i = 0; i < 40; i++)
        {
            if(dungeon[i][j] == 9 && dungeon[i - 1][j] == 13 && dungeon[i][j - 1] == 14)
            {
                dungeon[i][j] = 7; // set broken tiles to dirt instead of stalagmite
                dungeon[i][j - 1] = 7;
                dungeon[i-1][j] = 7;
            }
        }
    }

Additionally, here are seeds for Catacombs that contain the broken wall tile:

550
2346
2377
2992
5162
6365
6500
6711
7152
7155
7462

And the code to fix it:

            if(dungeon[i][j] == 15 && dungeon[i][j + 1] == 1) { /// add this check
                dungeon[i][j + 1] = 8; // change left wall to left corner
            }

Last but not least, some more seeds with the overflow bug in DRLG_L1 documented by @mewmew:

24627
29946
32559
ssjkakaroto commented 5 years ago

Hi all, the latest changelog for The Hell 2 had me raise an eyebrow:

Fixed in-game menu animations, they would stop after machine spent 49+ days without rebooting (a rare original D1 bug, but we fix them all anyways);

Were you guys able to find this in the cleanup?

AJenbo commented 5 years ago

Yeah, tied to how the Windows API works, for DevilutionX it's instead 49 days after starting the game, so much less likely to happen. https://docs.microsoft.com/en-us/windows/desktop/api/sysinfoapi/nf-sysinfoapi-gettickcount

ghost commented 5 years ago

so much less likely to happen.

Not on my watch!

There is also another annoying time bug where once you hit the year 2038 the random function overflows and returns a value past 0x7FFFFFFF (a negative number) and thus all randomness ceases and dungeon is the same.

mewmew commented 5 years ago

There is also another annoying time bug where once you hit the year 2038 the random function overflows and returns a value past 0x7FFFFFFF (a negative number) and thus all randomness ceases and dungeon is the same.

Really? Have you tried it by setting the clock? Haha, would be a somewhat incredible way to cheat the PRNG :dagger:

ghost commented 5 years ago

I time traveled instead to 2038. Unfortunately there's an apocalypse in the future and my time machine busted so its parts went everywhere and I had to walk back. We will see them one day! Next time I'll take you with me.

AJenbo commented 5 years ago

@ssjkakaroto i'm pretty sure this is the affected code: https://github.com/diasurgical/devilution/blob/8f40c35eabb72dcea5b1a478d893779a9fee6b51/Source/gmenu.cpp#L165

But looking at the code there appears to be a ton of things that would be affected by this, not being able to join a game, battle.net chat not working, lava not animating, sounds not playing, multiplayer stopping, quest dialogs not closing automatically, game can hang on launch if you hit the exact moment (and have an old version of MS Office installed), the game doesn't detect network disconnect, some in-game chat messages not being displayed, targeting other players being interrupted working, game crashes if video context is lost during the rollover. Package drop if sent around the rollover.

Some of the bugs may only appear to happen at the exact moment of the rollover. Also there's a related but that autosave won't work after 25 days.

So yeah, menu animations are probably the least of your worries if you play the game for more then 50 days at a time :)

AJenbo commented 5 years ago

Dirt tiles can appear on top of doors

image

ghost commented 5 years ago

Seems to me the issue is that the tile isn't set to be transparent.

ghost commented 5 years ago

The file gendata/logo.smk is broken in the original release. The first 25 frames have garbage filling the background instead of a black color. Though you can't notice it unless you have Windows 7. It seems this was fixed in all diablo releases after 1998, only the original 1996 discs have the broken file.

Checksums:

(Broken) MD5: 68f049866b44688a7af65ba766bef75a    Date: 12-26-1996
(Fixed) MD5: 011bc6518e6166206231080a4440b373    Date: 12-04-1997

Animated GIFs:

Broken Fixed

ghost commented 5 years ago

Proper documentation for the bug outlined here: https://github.com/diasurgical/devilutionX/issues/477 This bug is caused by the DRLG_L3Pool function not filling all blank tiles with lava tiles. Here a few sample seeds to see it on level 9:

751
1222
2617
3031
7675
8117
8616

Here is a pic of seed 2617 DIABLO_20191117_001530

AJenbo commented 4 years ago

If you load a save game where Diablo is in his Death throes the audio will be muted after the movies play. This happens because sgbSaveSoundOn is left unset since M_DiabloDeath is skipped.

ghost commented 4 years ago

The unique helmet Helm of Sprits has a typo, it was clearly meant to say Spirits. The PSX version translated it to say spirits in other languages, so was implied.

mewmew commented 4 years ago

To keep bugs in one place, I've inlined the comments of diasurgical/devilutionX#572 (Ground Tile Rendering Grey) reported by @mgpat-gm here:

From https://github.com/diasurgical/devilutionX/issues/572#issuecomment-581065521: image

From https://github.com/diasurgical/devilutionX/issues/572#issuecomment-581112877:

This is a hole in the town map, not something specific to DevilutionX and fixing it requires patching the data so out of scope for what we are doing atm. You can report original issues like this in the upstream bucket issue: https://github.com/diasurgical/devilution/issues/64

Chance4us commented 4 years ago

Next one vanilla bug. single_7.sv.zip vanilla

qndel commented 4 years ago

image that's how the quest panel should actually look but we NEVER saw the golden "quest log" because they messed up with the drawing order -

void DrawQuestLog()
{
    int y, i;

    PrintQLString(0, 2, TRUE, "Quest Log", 3);
    CelDraw(SCREEN_X, SCREEN_Y + 351, pQLogCel, 1, SPANEL_WIDTH);

Quest Log gets drawn first then gets covered by the panel, swap these 2 lines to fix

ghost commented 4 years ago

Old remains from the demo when they used the store dialog for quests: ql

qndel commented 4 years ago

monsters.cpp -> MonstStartKill

if (pnum < MAX_PLRS && i > MAX_PLRS)
        AddPlrMonstExper(Monst->mLevel, Monst->mExp, Monst->mWhoHit);`

i > MAX_PLRS should be i >= MAX_PLRS, otherwise monsters at index 4 (which are perfectly valid) don't give xp - this happens to be butcher/leoric etc. and I'm assuming on levels without quest monsters, there's 1 random monster that doesn't give xp Discovering new d1 bugs in 2020 = yay :D

AJenbo commented 4 years ago

Off-by-one pixel when drawing line in unique item box and stores box (see https://github.com/diasurgical/devilution/commit/36e319cb7f005e5db1fe078a7a102e10292b9840 and https://github.com/diasurgical/devilution/pull/2056/commits/6448f7901bc0ac4740a793f96f132ef9291e0bfc)

mgpat-gm commented 4 years ago

According to https://diablo.fandom.com/wiki/Rogue:

There is a bug that causes rogues to gain 3 mana instead of 2 when reaching the 33rd level

I have not verified, but it sounds plausible.

According to JG 1.62:

As a curiosity, the light radius is always one square less in the catacombs and it is always the highest light radius you have had on a level that counts, even if you later lower it.

Edit: removed the portion about rounding down.

NiteKat commented 4 years ago

On "it is always the highest light radius you have had on a level that counts, even if you later lower it." this has not been observed in testing. Xavier has done experiments with Diablo where he has parked Diablo on a specific tile, and approached with no minus light radius gear. Found the tile that activates Diablo, reloads, and marks that tile with gold. He then equips a minus light radius item (while not leaving the level), and can now go to that same tile and Diablo no longer activates.

mgpat-gm commented 4 years ago

On "it is always the highest light radius you have had on a level that counts, even if you later lower it." this has not been observed in testing. Xavier has done experiments with Diablo where he has parked Diablo on a specific tile, and approached with no minus light radius gear. Found the tile that activates Diablo, reloads, and marks that tile with gold. He then equips a minus light radius item (while not leaving the level), and can now go to that same tile and Diablo no longer activates.

I'm not sure it applies to level 16.

always one square less in the catacombs

I have not tested yet.

NiteKat commented 4 years ago

I'm not sure it applies to level 16.

Hm. Level 16 is the easiest place to test this (because Diablo can be parked at certain points with relative ease, and is always only the floor), so that's where I've seen this tested. I'll bring this up with Xavier (with Diablo) and see if they can do similar tests on other floors.

mewmew commented 4 years ago

There are two typos made in the missile direction index lookup quadrant in GetDirection16 which results in a non-symmetric division of space between the various directions. This is better illustrated with a screenshot.

bugfix_GetDirection16

qndel commented 4 years ago

think this is the reason why flash is missing on some tiles?