Skip to content

Custom display stats #5752

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

Open
wants to merge 25 commits into
base: dev
Choose a base branch
from

Conversation

Regisle
Copy link
Member

@Regisle Regisle commented Mar 4, 2023

This is a very technical change too make it easier to support custom display stats in the future

For now this allows users to save/load custom display stats to settings, though there is no UI to do so and thus requires users to add them manually, this PR is mainly meant to make it easier to add a UI in future to allow users to easily customise it

eg somthing like

<StatList>
		<statGroup index="1">
			<stat label="Total Life" displayStat="true"/>
			<stat label=" Total Life" minionDisplayStat="true"/>
			<stat label="Unreserved Life" displayStat="true"/>
			<stat label="Life Recoverable" displayStat="true"/>
			<stat label="Life Regen" color="^x7070FF" compPercent="true" displayStat="true" minionDisplayStat="true"/>
		</statGroup>
	</StatList>

looks like
image
and gives percentage change when hovering over regen

the amount of data needed to be saved is reduced by the fact that most of the data is reused from the old display stats if the stat shares the same label, overriding any keys with ones saved in settings (as well as displayStat/minionDisplayStat/extraSaveStat)

@QuickStick123 QuickStick123 added the technical Hidden from release notes label Mar 4, 2023
Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

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

I really don't like the magic space character differentiating the stats. I see the problem you're trying to solve, but at the same time you're adding the space where it otherwise wouldn't have to exist (e.g. "Life Cost" being for LifeCost and LifePercentCost).

Seems like there's a fine differentiator for most cases in "minionDisplayStat". For the few other duplicates, we could just add another property, most likely.

@Regisle
Copy link
Member Author

Regisle commented Mar 4, 2023

I really don't like the magic space character differentiating the stats. I see the problem you're trying to solve, but at the same time you're adding the space where it otherwise wouldn't have to exist (e.g. "Life Cost" being for LifeCost and LifePercentCost).

This is because there are a few stats which have different conditions for showing like "show average", or need different labels like "attack speed" and "cast speed", and I cant tinker with the stat, I could have kept reference to the index or a count of number of that stat or something, but all of those kinda references would break if we shuffled or added things, and I wanted it to be future proof, this leaves me with complex things like adding a number or tag and doing checks which endup way more complex than just adding a leading space which does nothing display-wise because its aligned by the end of the string not the start

Seems like there's a fine differentiator for most cases in "minionDisplayStat". For the few other duplicates, we could just add another property, most likely.

I do not want to differentiate by that becouse I want to allow people to add whatever stats they want to either player or minions, eg some people might want to see more defensive stats for minions, or something like chaos res was important when fleshcrafter was common

worst case is I can trim leading spaces when adding it to display/minion display

@Regisle Regisle force-pushed the customDisplayStats branch from f02c9cd to 994123f Compare April 3, 2023 00:01
@Regisle Regisle force-pushed the customDisplayStats branch from 994123f to 7b74829 Compare March 28, 2024 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical Hidden from release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants