Remove 3x Demon damage VS Golem

[kphoenix137]

Golem is a Demon class monster, so items like Deadly Hunter and Civerb's Cudgel will deal massive damage to it. I have address this in a previous PR, however that PR also added highlighting. The second part might be a bit more controversial, so this PR is narrowed down to what I believe is necessary.

I don't believe that the Golem was intentionally meant to be dealt 3x damage by these demon damaging weapons. Golem is a monster that you can never highlight, which shows the original developers' intent that this isn't a monster that you want to intentionally target and do damage to. Since this would have been the only monster to be truly classless, I believe they settled on Demon for a couple reasons.

1. Undead would allow the Golem to intercept Holy Bolts rather than letting them pass through, which would be a weird interaction. The Golem would also take more damage from blunt weapons, however this is nowhere as bad as the Holy Bolt problem.
2. Animal would have allowed Golem to be targeted and damaged by the Doom Serpents spell, which would infinitely redirect and do collision with animal type monsters multiple times until its timer ran out. It's possible that Golem could have existed at the same time as Doom Serpents during the development cycle, prompting them to intentionally not select animal as Golem's class. Also, the Golem would take more damage from blades, however like before, that's not as important as the previous problem.
3. Demon poses no problems, other than taking much more damage from only 3 weapons in the game, and no strange interactions with any spells. This makes it the most suitable candidate for the Golem's class without having to create a new enumerator for the Golem only.

It could be argued that they would have added logic to prevent the Golem from taking this 3x damage in missiles.cpp, however it's unlikely they ever found a scenario where this became apparent there was this interaction, considering there were only 2 working weapons with this item power, and both were melee, making it very unlikely that they were damaging their Golem in normal combat.

On a final note, we know that Diablo takes heavy inspiration from D&D. In D&D, Golems are considered to be "constructs", rather than animal, undead, or demon.

[julealgon]

...without having to create a new enumerator for the Golem only.

What about doing this? Is it not feasible? Add a new monster category, say, "Magical" (or "Construct" like you mentioned, or something else similar) and assign that to the Golem.

It would avoid the special casing in the demon bonus damage formula there.

[kphoenix137]

...without having to create a new enumerator for the Golem only.

What about doing this? Is it not feasible? Add a new monster category, say, "Magical" (or "Construct" like you mentioned, or something else similar) and assign that to the Golem.

It would avoid the special casing in the demon bonus damage formula there.

Not sure if this would have some kind of vanilla reverse compatibility implications with single player. It seems easier to just stop the 3x damage from occurring, which fixes the issue. In the case we revisit the PR to highlight the Golem, such a change would be necessary since monster class can be displayed, however there's no guarantee that will ever happen.

[julealgon]

It seems easier to just stop the 3x damage from occurring, which fixes the issue.

Well, it sure is easier, no doubt, but it is terrible for the maintainability of the game. If you keep adding special cases like this to the codebase it becomes a mess really quickly.

I'd not push this unless it really is currently unfeasible to add a new monster type and assign that to the golem.

In the case we revisit the PR to highlight the Golem, such a change would be necessary since monster class can be displayed, however there's no guarantee that will ever happen.

I don't see how that's related at all: they seem orthogonal to me. Can you elaborate?

[kphoenix137]

is easier, no doubt, but it is terrible for the maintainability of the

I had another PR where I added highlighting for the golem, which displays information about it in the infostring and health bar, like with monsters. This causes it to display that it's a Demon type to the player, which if is considered not ideal (related to the problems I had with Golem being a Demon, and why I believe it was chosen out of ease rather than Golem intentionally being considered a Demon), then it would need to either have it's type removed/replaced visually, or a new enumerator for a Golem specific type.

Anyway, I reviewed Devilution code, and it looks like this shouldn't break savegames. Also, the monster class isn't saved to the savegame but is always looked up in the monster data table. So yeah, you're right that it should be fixed via adding an enumerator.

I decided on Neutral, since this class would have no strengths/weaknesses against any type of spell or weapon. I would have settled on Construct, however that seems far too specific and arbitrary. Neutral may be less confusing to people viewing the code.

[julealgon]

I decided on Neutral, since this class would have no strengths/weaknesses against any type of spell or weapon.

I don't think the name should be based on the behaviors behind it. Instead, the name should be as semantic as possible based on the ingame representation.

Keep in mind we could actually add special behaviors for this type later, as well. Naming it "Neutral" based on the fact it currently has nothing special about it is not future-proof design.

I would have settled on Construct, however that seems far too specific and arbitrary. Neutral may be less confusing to people viewing the code.

Construct would be a vast improvement over Neutral to me, since, again, it's supposed to be in-game lore consistent and not for "people viewing the code".

However, I'll leave it to other folks to validate the name.

[kphoenix137]

I decided on Neutral, since this class would have no strengths/weaknesses against any type of spell or weapon.

I don't think the name should be based on the behaviors behind it. Instead, the name should be as semantic as possible based on the ingame representation.

Keep in mind we could actually add special behaviors for this type later, as well. Naming it "Neutral" based on the fact it currently has nothing special about it is not future-proof design.

I would have settled on Construct, however that seems far too specific and arbitrary. Neutral may be less confusing to people viewing the code.

Construct would be a vast improvement over Neutral to me, since, again, it's supposed to be in-game lore consistent and not for "people viewing the code".

However, I'll leave it to other folks to validate the name.

Fair enough, I can change it to construct. And we'll go from there.

[AJenbo]

I don't really see the need for this, it's not clear why other then being aesthetically pleasing to players of D&D.

[kphoenix137]

I don't really see the need for this, it's not clear why other then being aesthetically pleasing to players of D&D.

A need for the PR altogether, for for a special name for the Golem's class? Originally I went with a special case to prevent 3X demon damage from affecting the Golem, but julealgon recommended against it and instead making a new enumerator for the class.

[AJenbo]

Ok, but why does it need to not receive 3x damage?

[kphoenix137]

Ok, but why does it need to not receive 3x damage?

Because it's not supposed to :D (I think)

[AJenbo]

I'm getting to much maybe vibe from this, sorry. If you think I'm wrong hit me up on the chat.