Skip to content

Consolidate object declaration name conventions - #37322

Open
kwotaq wants to merge 2 commits into
ppy:pp-devfrom
kwotaq:object-declaration-pvp
Open

Consolidate object declaration name conventions#37322
kwotaq wants to merge 2 commits into
ppy:pp-devfrom
kwotaq:object-declaration-pvp

Conversation

@kwotaq

@kwotaq kwotaq commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

This pr aims to unify all the variable names for object declarations in the various evaluators and skills for the osu ruleset. To achieve better clarity in the naming the casting for the object types has been moved outside the evaluators and into the skills, which also unified some more use cases like the strain decay delta times. The chosen naming convention is up for debate, especially the ones regarding loop objects.

Comment thread osu.Game.Rulesets.Osu/Difficulty/Evaluators/Aim/FlowAimEvaluator.cs Outdated
Comment thread osu.Game.Rulesets.Osu/Difficulty/Evaluators/ReadingEvaluator.cs
@peppy

peppy commented Jul 2, 2026

Copy link
Copy Markdown
Member

I disagree with the new naming.

prev1Obj sounds weird. And I don't see why we need to use currObj for the parameter.

The only thing I'd propose is renaming current to obj.

@kwotaq

kwotaq commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

I disagree with the new naming.

prev1Obj sounds weird. And I don't see why we need to use currObj for the parameter.

The only thing I'd propose is renaming current to obj.

While prev1Obj seems weird at first its more consistent than having to go prevprev an object back and then switch to prev2 when you have to go further. As for currObj, right now current's entire existence is in the evaluator is just to be typecast into an OsuDifficultyHitObject and bloat rider's autofill function when you type curr. Having it just renamed to obj gives us 0 context about it and is more confusing in my opinion.

@kwotaq
kwotaq force-pushed the object-declaration-pvp branch from 50da292 to 8adf426 Compare July 27, 2026 14:30
wideAngleBonus *= Math.Min(wideAngleCurrVelocity, wideAnglePrevVelocity);

if (osuLast2Obj != null)
if (prev2Obj != null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how about prevObj2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah that sounds fine

flashlightDifficulty += stackNerf * opacityBonus * scalingFactor * jumpDistance / cumulativeStrainTime;

if (currentObj.Angle != null && osuCurrent.Angle != null)
if (loopObj.Angle != null && currObj.Angle != null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for loops i'd prefer just o or something if we can.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Can't say I agree with this given most of the times we end up comparing it with other objects in the loop and having it just be "o" makes it confusing really fast. In this evaluator we also have the HitObject of the loopObj so what would that be named if we change it to o, oHitObject?

@peppy

peppy commented Aug 14, 2026

Copy link
Copy Markdown
Member

Should I make my proposed changes and get this merged? It's going to conflict pretty fast if not acted on. cc @stanriders @tsunyoku

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Pending Review

Development

Successfully merging this pull request may close these issues.

5 participants