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.

Sign up for GitHub

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

Jump to bottom

Convert the health part of the mobcritter hud to use the hudzones system #18677

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Convert the health part of the mobcritter hud to use the hudzones system #18677

wants to merge 5 commits into from

Conversation

ZeWaka
Copy link
Member

@ZeWaka ZeWaka commented Apr 29, 2024

About the PR

title, the top right health area now uses the hudzones system

still need to do the weird stuff like rads, oxy, etc WIP

Why's this needed?

More clean and modern system, infinitely less jank, able to dynamically position elements

@ZeWaka ZeWaka added C-Code-Quality Cleans up code, refactors things to be more readable or intuitive C-Rework Reworks a feature A-UI Modifies UI in some way. Automatically applied on a change to tgui/ A-Critters Deals with any of our NPC critters - on or off-station. labels Apr 29, 2024
@boring-cyborg boring-cyborg bot added A-Tooling Changes tooling for the repository. C-Documentation Deals with *.md files or other docs labels Apr 29, 2024
@github-actions github-actions bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict labels Apr 29, 2024
@ZeWaka ZeWaka added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 29, 2024
mordent-goonstation
mordent-goonstation reviewed May 3, 2024
View reviewed changes
_std/macros/qdel.dm
#define QDEL_NULL(item) qdel(item); item = null

/// qdel every item in a list, then cut the list
#define QDEL_LIST(L) if(L) { for(var/I in L) qdel(I); L.Cut(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define QDEL_LIST(L) if(L) { for(var/I in L) qdel(I); L.Cut(); }
#define QDEL_LIST(L) if(L) { for(var/I as anything in L) qdel(I); L.Cut(); }

Dunno if it matters in this case given lack of type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's equivalent yeah

code/atom.dm
@@ -518,7 +518,7 @@ TYPEINFO(/atom/movable)
var/mechanics_type_override = null

//some more of these event handler flag things are handled in set_loc far below . . .
/atom/movable/New()
/atom/movable/New(loc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh how horrifying that this is just being introduced. :)

code/datums/controllers/process/machines.dm Outdated Show resolved Hide resolved
code/datums/hud/critter.dm
if(10 to 20)
screen_obj.icon_state = "health5" // red
else
screen_obj.icon_state = "health6" // crit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is parity with existing, but noting for future discussion that it's weird that you show as "crit" for the last 10% of your health, right? I don't think you're actually in crit at that point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For critters you pretty much are, since you die at 0% instantly. That was how I always read it.

code/mob/living.dm
@@ -134,10 +134,6 @@
src.ensure_bp_list()

if (src.use_stamina)
//stamina bar gets added to the hud in subtypes human and critter... im sorry.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Begone, foul demon.

code/mob/living/stamina_stuns.dm
src.stamina_bar.update_value(src)
return

// kill me
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean... doesn't seem that awful.

code/mob/living/stamina_stuns.dm
if(src.stamina_bar) src.stamina_bar.update_value(src)
return

// this will never be unified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean yeah, fair.

Stamina component when?

code/modules/interface/hudzones/hud_zones.dm
@@ -55,32 +55,32 @@
/datum/hud_zone/proc/adjust_offset(datum/hud_element/element)
PRIVATE_PROC(TRUE)
// prework
var/absolute_pos_horizontal // absolute horizontal position (whole screen) where new elements are added, used with hud offsets
var/relative_pos_horizontal // absolute horizontal position (whole screen) where new elements are added, used with hud offsets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment no longer right?

code/modules/interface/hudzones/hud_zones.dm

var/absolute_pos_vertical // absolute vertical position (whole screen) where new elements are added, used with hud offsets
var/relative_pos_vertical // absolute vertical position (whole screen) where new elements are added, used with hud offsets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment no longer right?

@ZeWaka @mordent-goonstation
Update code/datums/controllers/process/machines.dm
82c9d13 
Co-authored-by: Mordent <62817778+mordent-goonstation@users.noreply.github.com>
@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label May 8, 2024
@github-actions GitHub Actions
Copy link
Contributor

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label May 22, 2024
@ZeWaka
Copy link
Member Author

ZeWaka commented May 22, 2024

stfu im busy

@github-actions github-actions bot removed the S-Stale An inactive PR that has had no updates in the past two weeks label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MylieDaniels MylieDaniels MylieDaniels left review comments

@mordent-goonstation mordent-goonstation mordent-goonstation left review comments

Assignees
No one assigned
Labels
A-Critters Deals with any of our NPC critters - on or off-station. A-Tooling Changes tooling for the repository. A-UI Modifies UI in some way. Automatically applied on a change to tgui/ C-Code-Quality Cleans up code, refactors things to be more readable or intuitive C-Documentation Deals with *.md files or other docs C-Rework Reworks a feature S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ZeWaka @MylieDaniels @mordent-goonstation