Skip to content
This repository has been archived by the owner on Jun 10, 2021. It is now read-only.

Placeholder diff calc #12

Merged
merged 2 commits into from
Apr 6, 2021
Merged

Placeholder diff calc #12

merged 2 commits into from
Apr 6, 2021

Conversation

Beyley
Copy link

@Beyley Beyley commented Apr 5, 2021

Just shows the number of keys required, which in my experience with a bunch of maps roughly can show the basic difficulty, as the more keys, the more you have to keep track of
At the very least it stops everything from being 0

@Thesola10
Copy link
Owner

Goes towards work on #6 I suppose

@Thesola10 Thesola10 requested a review from Artemis-chan April 6, 2021 07:42
Comment on lines +26 to +35
if (od > 6.0d)
{
difficulty = 4;
} else if (od > 4.5d)
{
difficulty = 3;
} else if (od > 2d)
{
difficulty = 2;
}
Copy link
Owner

Choose a reason for hiding this comment

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

please bear with me for a sec, but what is calculating OverallDifficulty and why can't its value simply be propagated as-is to the DifficultyAttributes constructor?

Copy link
Author

Choose a reason for hiding this comment

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

OverallDifficulty is pulled from the standard map, as thats the only maps we have currently, since they are all converts

Copy link
Collaborator

@Artemis-chan Artemis-chan Apr 6, 2021

Choose a reason for hiding this comment

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

i think it just makes it so the od of standard map and difficulty for this mode is not same, i think it works for now

Copy link
Owner

Choose a reason for hiding this comment

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

oh right, that works until #9 is worked on
but then why not just multiply it by sth like 0.6 to keep those sweet float values?

Copy link
Collaborator

@Artemis-chan Artemis-chan Apr 6, 2021

Choose a reason for hiding this comment

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

oh i just noticed it shows the number of keys to be used in the map

Copy link
Owner

Choose a reason for hiding this comment

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

oh i just noticed it shows the number of keys to be used in the map

Mania doesn't do that, so is it really necessary for us?

Copy link
Author

Choose a reason for hiding this comment

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

oh i just noticed it shows the number of keys to be used in the map

Mania doesn't do that, so is it really necessary for us?

Its a start as not only does OD generally tell a very basic difficulty of a map in STD, but the more keys here makes the game exponentially harder

@Thesola10
Copy link
Owner

PS (entirely off-topic): you can upload your GPG key to GitHub to get your commits shown as Verified, using gpg --armor --export CB6C781B33E6BE52 and pasting the results on GitHub.com

@Beyley
Copy link
Author

Beyley commented Apr 6, 2021

PS (entirely off-topic): you can upload your GPG key to GitHub to get your commits shown as Verified, using gpg --armor --export CB6C781B33E6BE52 and pasting the results on GitHub.com

and yeah ive been putting it off for awhile lol

Copy link
Owner

@Thesola10 Thesola10 left a comment

Choose a reason for hiding this comment

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

We need to make sure this code doesn't come back to bite us in the future 👀

Co-authored-by: Karim Vergnes <[email protected]>
Copy link
Owner

@Thesola10 Thesola10 left a comment

Choose a reason for hiding this comment

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

This approach to diff calc is fine right now

@Thesola10 Thesola10 merged commit 35ec1e2 into Thesola10:master Apr 6, 2021
@Beyley Beyley deleted the difficulty-calculator branch April 6, 2021 10:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants