[pull] master from ppy:master #181
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
…"per-DifficultyHitObject"
and fixed one minor CI code quality thing
Update framework
Display breaks on editor timeline & allow manually adjusting their duration
CodeFileSanity doesn't like records and it being a record wasn't doing much anymore anyway.
Add verify checks for incorrect title markers
Reimplement missing gameplay test hotkeys from stable
Add ability to cycle slider control point types via keyboard
Change editor speed adjustment back to adjusting tempo
Fix editor not updating ruleset when switching difficulty
Always inherit the volume from the previous hit object on placement
Use room watching functionality to receive realtime daily challenge updates
Fix incorrect skin fallback order when beatmap skin is present
Disappeared in latest EAP and doesn't look like something we would have wanted to be committed in the first place.
Apply nullability to `MusicController`
Allow inheriting addition bank from previous hitobject when using auto bank assignment
Fix node samples always having default samples on placement
Closes #28741. Regressed in a7b066f. The intent of the original change there was to ensure that addition banks being set will put the ternary state toggles in indeterminate state (to at least provide a visual indication that the selection does not use a single bank). This would previously not be the case due to the use of `.All()` in the original condition (a single object/node was considered to have a bank enabled if and only if *all* samples within it used it). However the attempt to fix that via switching to `Any()` was not correct. The logic used in the offending commit operates on extracted `Samples` and `NodeSamples` from the selection, and would consider the ternary toggle: - fully off if none of the samples/node samples contained a sample with the given bank, - indeterminate if the some of the samples/node samples contained a sample with the given bank, - fully on if at least one sample from every samples/node samples contained a sample with the given bank. This is a *two-tiered* process, as in first a *binary* on/off state is extracted from each object's samples/node samples, and *then* a ternary state is extracted from all objects/nodes. This is insufficient to express the *desired* behaviour, which is that the toggle should be: - fully off if *none of the individual samples in the selection* use the given bank, - indeterminate if *at least one individual sample in the selection* uses the given bank, - fully on if *all individual samples in the selection* use the given bank. The second wording is flattened, and no longer tries to consider "nodes" or "objects", it just looks at all of the samples in the selection without concern as to whether they're from separate objects/nodes or not. To explain why this discrepancy caused the bug, consider a single object with a `soft` normal bank and `drum` addition bank. Selecting the object would cause a ternary button state update; as per the incorrect logic, there were two samples on the object and each had its own separate banks, so two ternary toggles would have their state set to `True` (rather than the correct `Indeterminate`), thus triggering a bindable feedback loop that would cause one of these banks to win and actually overwrite the other. Note that the addition indeterminate state computation *still* needs to do the two-tiered process, because there it actually makes sense (for a selection to have an addition fully on rather than indeterminate, *every* object/node *must* contain that addition).
Fix incorrect ternary state computation for bank toggles
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )