-
Notifications
You must be signed in to change notification settings - Fork 213
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 bar healthbars. #5399
base: master
Are you sure you want to change the base?
add bar healthbars. #5399
Conversation
#extension GL_ARB_uniform_buffer_object : require | ||
#extension GL_ARB_shading_language_420pack: require | ||
|
||
// This shader is (c) Beherith (mysterme@gmail.com) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a killer is using the shaders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beherith said he would release his shaders as MIT and there's beyond-all-reason/Beyond-All-Reason#3568 but it was somewhat ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. So it's a question if we want MIT shaders. I think just add the MIT headers to the files and asking Beherith to approve the PR, which would give us his consent to use it under MIT. Since the file doesn't currently have these licenses, it could be argued that they are not released under the license.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the license at this moment in time:
// This file is going to be licensed under some sort of GPL-compatible license, but authors are dragging
// their feet. Avoid copying for now (unless this header rots for years on end), and check back later.
// See https://github.com/ZeroK-RTS/Zero-K/issues/5328
I don't want to put anything that looks more like an actual license here until the BAR issue is resolved with an actual license.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll raise the topic at the next bar contributor meeting.
which should be on the second of January 2025
if not it will be the next thursday.
I am starting to understand this I need a lot more time. I have no background on GL shaders, so this is new to me. |
There is some sort of "Widget Profiler" widget available via Alt+F11, it is vrey flawed though. The shader approach may be an order of magnitude faster and visible there anyway, hard to tell. For proper, accurate profiling there's Tracy https://beyond-all-reason.github.io/spring/development/profiling-with-tracy but that one requires adding tracy scope points all over gameside code (it's free perf-wise in non-tracy engine builds but clutters up the code a bit) which looks like this https://github.com/search?q=repo%3Abeyond-all-reason%2FBeyond-All-Reason%20tracy&type=code |
Tracy is a good subproject, one that probably wants to be done before any large profiling efforts. I didn't know it required manual scoping though, that sounds like a lot of legwork. So perfect for a PR. On GL4 healthbars, I think we can safely assume that they are much faster than jK bars. The tricky part will be using the tech to make something that fits ZK. |
Editing the lua should be easy. Just copy the kj "getters" into the bar health bars, though it does handle them differently, keeping track of the status, similarly to how we do morph. I was thinking of doing something similar so units that are no longer in Los can have outdated health bars. That is a separate feature. |
@GoogleFrog Do you want me to work towards replacing the jk health bars and experimental gui_healthbars with bar healthbars? |
Yes, that would be good. The existing widget shouldn't be touched, unless perhaps you want to make a shared config file that controls both widgets. We'll probably want to use the existing widget as a shaderless fallback. The unused shader widget looks removable. |
Data is passed to to the shaders using |
Some notes:
|
I didn't realize that the uniform was shared between all shaders. That makes things much harder. If this is the case, I feel we should have a widget that caches and updates the uniform for all unit status. Other widgets can read the cache directly (for non shaders) or relay on the widget to update the uniform for their shaders. Once I get all the bars working, I can split this widget. |
Perhaps what we need is a uniform that tells the shader which bars are being drawn (via a bitmask?), and then four or five uniforms that contain the data to draw in each case. |
I think we can wait until we are closer to running out of channels before we resort to a bitmap of what the values are. We are using just over half. I think other shaders will need the status provided anyway. Most of the other shaders Sprung mentioned can easily use the status provided by the bar, or overload one already in use. If we want to flash bars based on unit selection and command, that info will also need to be sent. |
Connected more bars. The remaining bars to connect are: Morph, Reload, Dgun, Teleport, Ability, Stockpile. |
I've been thinking there should be a stable before Christmas, but after the tournament tomorrow. This PR is too large and late to get into that stable, but I should be able to look at it shortly after. We will want some sort of solid handling of which healthbars are enabled. Eg, setting the default to be disabled for the old bars won't disable them for existing players. I think there should be a setting for which bar the player wants enabled, and a fallback that enables the old bars if the player has insufficient shader support. The new setting should be a strong enough source of truth to disable the old bars for existing players. |
I agree, this is too large to be ready in a few days. I do plan on adding a bar style selection option. When I do this, I plan on making a plain shader less option. With this option, we can remove the jk bar widget. I have grand plans for the bars. The difficulty is deciding what is mvp. MVP IMO:
So there is still a lot to work on before this is ready. |
This PR is mainly to create discussion on the future of Zero-k healthbars.