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

add yellow outline to berserked monsters #3122

Closed
wants to merge 1 commit into from

Conversation

qndel
Copy link
Member

@qndel qndel commented Oct 13, 2021

and remove original buggy light
image

Resolves #1588
Resolves #1586

@qndel qndel added the enhancement New feature or request label Oct 13, 2021
@ThomasChr
Copy link
Contributor

We should either merge this PR or #3120 and #3121

@AJenbo
Copy link
Member

AJenbo commented Oct 13, 2021

We should either merge this PR or #3120 and #3121

I merged #3120 for now, it's a definit bug fix. For #3121 I would like to think about it for a bit since it's a change compared to the original, maybe it's best to wait til after 1.4.0, but my initial feeling is that I really like the looks of it.

@agris-codes
Copy link

agris-codes commented Oct 14, 2021

Why use a totally different visual indicator rather than just fixing the original indicator? The sprite outline looks totally out of place to me.

@ThomasChr
Copy link
Contributor

@agris-codes The original indicator (the light source for a berserk monster) has been fixed with my two PRs. The problem is that the light source only allows for 32 light sources in total. And we can‘t change that easily because of savegame compatibility.
Also the original indicator (the light source) was sometimes hard to see.
I think a new indicator is really better. But yes, it‘s a noticable change and up for discussion.

@FitzRoyX
Copy link

Why use a totally different visual indicator rather than just fixing the original indicator?

Because light radiance was already used to help unique monsters stand out from regular monsters. Hellfire trashes that by letting the player hand out light sources like candy.

@AJenbo
Copy link
Member

AJenbo commented Oct 14, 2021

The problem is that the light source only allows for 32 light sources in total. And we can‘t change that easily because of savegame compatibility.

That issue is largely fixed with your PR where it adds the light at load time, we could allow for more then 32 lights, and not save berserk lights in addition to avoid this being an issue at all (berserkers would not light up if loaded in the original game, would already be the case if they hit the limit)

Because light radiance was already used to help unique monsters stand out from regular monsters. Hellfire trashes that by letting the player hand out light sources like candy.

This was already an issue in Diablo since it also happens with firewalls.

@qndel
Copy link
Member Author

qndel commented Oct 14, 2021

The problem is that the light source only allows for 32 light sources in total. And we can‘t change that easily because of savegame compatibility.

That issue is largely fixed with your PR where it adds the light at load time, we could allow for more then 32 lights, and not save berserk lights in addition to avoid this being an issue at all (berserkers would not light up if loaded in the original game, would already be the case if they hit the limit)

Because light radiance was already used to help unique monsters stand out from regular monsters. Hellfire trashes that by letting the player hand out light sources like candy.

This was already an issue in Diablo since it also happens with firewalls.

Yes but no light source was permanent like hellfire's berserk, firewalls expire etc.

@AJenbo
Copy link
Member

AJenbo commented Oct 14, 2021

I see that as a separate issue, that has been fixed by removing the light on death.

@FitzRoyX
Copy link

FitzRoyX commented Oct 14, 2021

This was already an issue in Diablo since it also happens with firewalls.

Missile light is fine and diminishes nothing. A berserked monster sort of looks like a unique, especially if it's in an adjacent room and all you see is the light source moving around. It's not a good idea for any regular monster to be able to glow in darkness as a spell effect.

@ThomasChr
Copy link
Contributor

ThomasChr commented Oct 14, 2021

That issue is largely fixed with your PR where it adds the light at load time, we could allow for more then 32 lights, and not save berserk lights in addition to avoid this being an issue at all (berserkers would not light up if loaded in the original game, would already be the case if they hit the limit)

@AJenbo Should we change that bevore 1.3.0?
MAXLIGHT is only a define in lighting.h

@AJenbo
Copy link
Member

AJenbo commented Oct 14, 2021

No, increasing the lights is to risky, but we can do it right after release. Feel free to prepare a PR for it if you like.

@ThomasChr
Copy link
Contributor

ThomasChr commented Oct 14, 2021

I agree. Will prepare a PR (which will be very easy), and test it (which will take a little bit of time)...

@AJenbo
Copy link
Member

AJenbo commented Nov 6, 2021

Imo we solved the baserk lighting pretty well

@AJenbo AJenbo closed this Nov 6, 2021
@agris-codes
Copy link

Imo we solved the baserk lighting pretty well

Agreed, and I didn't intend to be dismissive of qndel's proposed fix. The sprite outline looked good technically, but was not a good aesthetic fit. Glad its resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Berserk light gets lost when reentering the level Berserk light bug
7 participants