Fix breath statbar scaling; defer breath_bar hiding by one second #8271

ClobberXD · 2019-02-22T07:12:09Z

PLAYER_MAX_BREATH_DEFAULT is set to 11 instead of 10, probably so that all 10 bubbles are shown before the breath bar disappears. See #8210 (comment) for more details.

This PR sets PLAYER_MAX_BREATH_DEFAULT to 10, and hides the breath bar after a delay of one second, so that all 10 bubbles are visible.

Fixes #8210. Tested.

To test:

Start losing breath in a drown-able liquid
Regain breath
Observe that all 10 bubbles are shown, before the breath bar disappears a second later

ClobberXD · 2019-02-22T07:24:42Z

At the moment, the breath bar does disappear once full. Should I make it permanently visible?

nerzhul · 2019-02-22T08:05:50Z

breath bar always disappear when full :)

ClobberXD · 2019-02-22T08:39:40Z

breath bar always disappear when full :)

I know, but doesn't that seem inconsistent with the health bar? If this behaviour is correct, shouldn't the health bar disappear when the player has full health as well?

nerzhul · 2019-02-22T08:42:28Z

i don't think. Health should be shown every time like in many RPG, and breath is only shown when consumed generally, the current behaviour is fine and natural for me

paramat · 2019-02-22T19:08:55Z

Breath bar disappearing is ok for me, less visual obstruction.

ghost · 2019-02-22T20:42:09Z

it's humorous to me that we are going from showing one half too many bubbles to showing one half too few. an improvement nonetheless.

…

On Fri, Feb 22, 2019, 2:09 PM Paramat ***@***.***> wrote: Breath bar disappearing is ok for me, less visual obstruction. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8271 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWXKYj_yeProlGacT-zfb3oHdy0l7t_mks5vQEBMgaJpZM4bJAEB> .

SmallJoker · 2019-02-23T11:28:48Z

Usually you don't care about breath, thus hiding it frees some space for the relevant game screen. I'd like to keep the old behaviour; internally the maximal value could be set to 1 higher so that the HUD may disappear correctly.

ClobberXD · 2019-02-23T13:28:54Z

Makes sense. What do you suggest I do now?

builtin/game/statbars.lua

paramat · 2019-02-23T19:46:27Z

I don't think 10 bubbles should ever appear, if 10 is full then 10 should be invisible. No bar means 10. This 11 thing is silly and messy.

ghost · 2019-02-23T21:35:42Z

I disagree. Having the bubbles statbar disappear when only at nine bubbles looks awkward. I suggest you try it and see for yourself. [Edit: For the record, I don't really think it matters overall, just giving my two cents.]

…

On Sat, Feb 23, 2019 at 2:46 PM Paramat ***@***.***> wrote: I don't think 10 bubbles should ever appear, if 10 is full then 10 should be invisible. No bar means 10. This 11 thing is silly and messy. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8271 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWXKYjB8bk6ED4aieQJ8JffhMN-cpAeoks5vQZqZgaJpZM4bJAEB> .

paramat · 2019-02-24T20:26:50Z

I don't need to try it i can imagine from current behaviour.
Currently, when at 10 bubbles there is still another breath regain cycle as the 11th is gained, then it disappears at 11. I can see why it was done but it was a bad and messy decision because 11 is a weird and inconsistent number.

ghost · 2019-02-24T20:51:46Z

it disappeared at ten, you never saw eleven.

…

On Sun, Feb 24, 2019, 3:26 PM Paramat ***@***.***> wrote: I don't need to try it i can imagine from current behaviour. Currently, when at 10 bubbles there is another breath regain cycle as the 11th is gained, then it disappears at 11. I can see why it was done but it was a bad and messy decision because 11 is a weird and inconsistent number. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8271 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWXKYq8CMA_2dgYRaWONO6_RPztiJtXJks5vQvWPgaJpZM4bJAEB> .

paramat · 2019-02-26T21:59:45Z

I know, that's what i wrote.

ghost · 2019-02-26T22:48:36Z

You wrote that ten shouldn't be visible, which would be a change in current behavior.

ClobberXD · 2019-02-27T01:42:40Z

SmallJoker's suggestion of reducing nominal by 1 doesn't seem like the best long-term solution, as there's still quite a bit of wizardry going on behind the scenes. While we can take this to be a quick-fix for 5.0.0, we could add proper support for statbars to automatically disappear when they are full, in 5.1.0. Support in the sense, there won't be the need to use magical numbers to make sure 10 bubbles appear, and the engine would take care of the hiding part without any intervention by us. :)

paramat · 2019-03-01T19:12:55Z

Yes i was proposing 10 not being visible, a change of behaviour.

Yes a more careful implementation is probably needed.
This PR won't be merged for 5.0.0 as it's a low-priority visual-only issue.

ClobberXD · 2019-04-26T16:05:51Z

Any thoughts or suggestions as to what's the best way to do this?

ClobberXD · 2019-08-02T05:02:54Z

On printing a bunch of values, I noticed that the player's breath_max is 11, in spite of having changed PLAYER_MAX_BREATH_DEFAULT to 10. MTG doesn't seem to set breath_max to any value either. This can't be due to old modifications I made when testing this PR, as it's 11 in a new world as well. I'll see if this is initialized somewhere in the engine...

ClobberXD · 2019-08-02T07:38:41Z

It seems like I just didn't re-compile after setting PLAYER_MAX_BREATH_DEFAULT to 10. It works now. All 10 bubbles are displayed, and if the bar is full, it is hidden one second later.

This PR now works, and is ready for review. :)

@SmallJoker @paramat

builtin/game/statbars.lua

ClobberXD · 2019-08-03T16:21:47Z

PR's ready for review (again). Tested by toggling breathbar HUD flag and immortal, and the statbar disappeared as expected. However, I did notice a delay of more than a second before the statbar disappeared due to the aforementioned actions. This is probably because there's no straight way to detect whether HUD flags or armor groups change, but I just wanted to put it out here.

SmallJoker

Works on my machine.

PLAYER_MAX_BREATH_DEFAULT was earlier set to 11, so that 10 bubbles are shown before the breath bar disappears. Now, PLAYER_MAX_BREATH_DEFAULT is set to 10, and the breath_bar scaling code in builtin has been tweaked to show all 10 bubbles before hiding the breath_bar

ClobberXD · 2019-08-21T14:28:39Z

Rebased.

ClobberXD · 2019-10-13T09:53:41Z

Bumpity-bump

ClobberXD · 2020-01-02T06:31:27Z

Bump

Added testing instructions to the first post.

ClobberXD · 2020-01-18T10:27:12Z

@SmallJoker Merge?

paramat · 2020-01-19T00:16:21Z

Not really trivial enough for a 1-approval merge.

ClobberXD · 2020-04-16T06:38:59Z

Delays may cause even more delays due to the need for re-testing. Better to merge this IMHO, instead of letting it stagnate, lest we have more PRs modifying the same areas of code and breaking this PR in the process.

sfan5

Tested. Works as expected.

ClobberXD · 2020-04-28T01:41:45Z

Thanks for the review!

ClobberXD mentioned this pull request Feb 22, 2019

Breath statbar might show 10.5 bubbles for high breath values #8210

Closed

paramat added the Bugfix 🐛 PRs that fix a bug label Feb 22, 2019

SmallJoker reviewed Feb 23, 2019

View reviewed changes

builtin/game/statbars.lua Outdated Show resolved Hide resolved

ClobberXD force-pushed the statbars_fix branch from 6fc5e47 to a803d10 Compare February 27, 2019 02:06

rubenwardy force-pushed the master branch 2 times, most recently from 0e3b135 to 39c54e1 Compare June 21, 2019 00:46

ClobberXD force-pushed the statbars_fix branch 2 times, most recently from a0417b0 to 4b60dec Compare August 2, 2019 07:38

ClobberXD changed the title ~~Fix extra bubbles being displayed for high breath_max values~~ Fix breath_bar scaling; delay breath_bar hiding by one second Aug 2, 2019

ClobberXD force-pushed the statbars_fix branch 2 times, most recently from ca11b5c to 28006bd Compare August 2, 2019 08:03

ClobberXD changed the title ~~Fix breath_bar scaling; delay breath_bar hiding by one second~~ Fix breath statbar scaling; defer breath_bar hiding by one second Aug 2, 2019

SmallJoker reviewed Aug 2, 2019

View reviewed changes

builtin/game/statbars.lua Outdated Show resolved Hide resolved

SmallJoker reviewed Aug 2, 2019

View reviewed changes

builtin/game/statbars.lua Outdated Show resolved Hide resolved

SmallJoker reviewed Aug 2, 2019

View reviewed changes

builtin/game/statbars.lua Show resolved Hide resolved

ClobberXD force-pushed the statbars_fix branch from 28006bd to 8a2238f Compare August 3, 2019 10:27

SmallJoker approved these changes Aug 3, 2019

View reviewed changes

SmallJoker added @ Builtin One approval ✅ ◻️ labels Aug 3, 2019

sfan5 added the Rebase needed The PR needs to be rebased by its author. label Aug 12, 2019

ClobberXD force-pushed the statbars_fix branch from 8a2238f to 6037a68 Compare August 21, 2019 14:28

SmallJoker removed the Rebase needed The PR needs to be rebased by its author. label Aug 21, 2019

sfan5 self-requested a review April 16, 2020 10:15

sfan5 approved these changes Apr 27, 2020

View reviewed changes

sfan5 added >= Two approvals ✅ ✅ and removed One approval ✅ ◻️ labels Apr 27, 2020

SmallJoker merged commit a36c9c3 into minetest:master Apr 28, 2020

ClobberXD deleted the statbars_fix branch April 29, 2020 01:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix breath statbar scaling; defer breath_bar hiding by one second #8271

Fix breath statbar scaling; defer breath_bar hiding by one second #8271

ClobberXD commented Feb 22, 2019 •

edited

ClobberXD commented Feb 22, 2019

nerzhul commented Feb 22, 2019

ClobberXD commented Feb 22, 2019

nerzhul commented Feb 22, 2019

paramat commented Feb 22, 2019

ghost commented Feb 22, 2019 via email

SmallJoker commented Feb 23, 2019

ClobberXD commented Feb 23, 2019

paramat commented Feb 23, 2019

ghost commented Feb 23, 2019 via email •

edited by ghost

paramat commented Feb 24, 2019 •

edited

ghost commented Feb 24, 2019 via email

paramat commented Feb 26, 2019

ghost commented Feb 26, 2019

ClobberXD commented Feb 27, 2019 •

edited

paramat commented Mar 1, 2019

ClobberXD commented Apr 26, 2019

ClobberXD commented Aug 2, 2019

ClobberXD commented Aug 2, 2019 •

edited

ClobberXD commented Aug 3, 2019

SmallJoker left a comment

ClobberXD commented Aug 21, 2019

ClobberXD commented Oct 13, 2019

ClobberXD commented Jan 2, 2020

ClobberXD commented Jan 18, 2020

paramat commented Jan 19, 2020

ClobberXD commented Apr 16, 2020

sfan5 left a comment

ClobberXD commented Apr 28, 2020

Fix breath statbar scaling; defer breath_bar hiding by one second #8271

Fix breath statbar scaling; defer breath_bar hiding by one second #8271

Conversation

ClobberXD commented Feb 22, 2019 • edited

ClobberXD commented Feb 22, 2019

nerzhul commented Feb 22, 2019

ClobberXD commented Feb 22, 2019

nerzhul commented Feb 22, 2019

paramat commented Feb 22, 2019

ghost commented Feb 22, 2019 via email

SmallJoker commented Feb 23, 2019

ClobberXD commented Feb 23, 2019

paramat commented Feb 23, 2019

ghost commented Feb 23, 2019 via email • edited by ghost

paramat commented Feb 24, 2019 • edited

ghost commented Feb 24, 2019 via email

paramat commented Feb 26, 2019

ghost commented Feb 26, 2019

ClobberXD commented Feb 27, 2019 • edited

paramat commented Mar 1, 2019

ClobberXD commented Apr 26, 2019

ClobberXD commented Aug 2, 2019

ClobberXD commented Aug 2, 2019 • edited

ClobberXD commented Aug 3, 2019

SmallJoker left a comment

Choose a reason for hiding this comment

ClobberXD commented Aug 21, 2019

ClobberXD commented Oct 13, 2019

ClobberXD commented Jan 2, 2020

ClobberXD commented Jan 18, 2020

paramat commented Jan 19, 2020

ClobberXD commented Apr 16, 2020

sfan5 left a comment

Choose a reason for hiding this comment

ClobberXD commented Apr 28, 2020

ClobberXD commented Feb 22, 2019 •

edited

ghost commented Feb 23, 2019 via email •

edited by ghost

paramat commented Feb 24, 2019 •

edited

ClobberXD commented Feb 27, 2019 •

edited

ClobberXD commented Aug 2, 2019 •

edited