Skip to content

Instantly share code, notes, and snippets.

@bdach

bdach/onclick-event-refactoring.md

Last active September 5, 2019 22:28
Show Gist options
  • Save bdach/96129a54f9454c959e6747965d465fb5 to your computer and use it in GitHub Desktop.
Save bdach/96129a54f9454c959e6747965d465fb5 to your computer and use it in GitHub Desktop.
Download ZIP
OnClick event refactoring
Raw
onclick-event-refactoring.md

OnClick event refactoring

Intended to resolve #5916.

Steps necessary for completion:

  • Change EnableClick to true in OsuUserInputManager.RightMouseManager
  • Review all OnClick() usages, add guard clauses wherever necessary
  • Review all case ClickEvent usages, add guard clauses wherever necessary
  • Review all OnMouseUp() usages, replace workarounds for right clicks with OnClick() handling

OnClick() usages

The following table lists all the found usages of the OnClick() handler and specifies whether or not a guard clause is suggested, and the effect of that handler.

Usage Guard suggested? Handler effect
osu.Game.Graphics.Containers.OsuFocusedOverlayContainer Yes Leaves overlay
osu.Game.Graphics.Containers.DialogButton Yes Presses button
osu.Game.Graphics.UserInterface.ExternalLinkButton Yes Presses button
osu.Game.Graphics.UserInterface.HoverClickSounds No Receiving right (or all, really) clicks desired to play sounds and address #5916
osu.Game.Graphics.UserInterface.OsuAnimatedButton Yes Presses button
osu.Game.Graphics.UserInterface.OsuMenu Yes Presses menu items
osu.Game.Graphics.UserInterface.TwoLayerButton Yes Presses button
osu.Game.Overlays.BeatmapSet.BeatmapPicker.DifficultySelectorButton Yes Presses button
osu.Game.Overlays.Changelog.UpdateStreamBadge Yes Presses badge
osu.Game.Overlays.Direct.DirectPanel Yes Dismisses panel
osu.Game.Overlays.Direct.PlayButton Yes Presses button
osu.Game.Overlays.KeyBinding.KeyBindingRow No The handler just swallows events
osu.Game.Overlays.Music.PlaylistItem Yes Presses button
osu.Game.Overlays.Notifications.Notification Yes Dismisses notification, launches completion actions
osu.Game.Overlays.Profile.Sections.Kudosu.KudosuInfo No The handler just swallows events
osu.Game.Overlays.Profile.Sections.DrawableProfileRow No The handler just swallows events
osu.Game.Overlays.Profile.Sections.ShowMoreButton Yes Presses button
osu.Game.Overlays.Settings.Sections.General.LoginSettings No The handler just swallows events
osu.Game.Overlays.Settings.Sections.General.LoginSettings.LoginForm No The handler just swallows events
osu.Game.Overlays.Settings.SettingsItem<T>.RestoreDefaultValueButton Yes Reverts setting to default
osu.Game.Overlays.Toolbar.ToolbarButton Yes Presses button
osu.Game.Overlays.BeatmapSetOverlay Yes Dismisses overlay
osu.Game.Overlays.MedalOverlay Yes? Dismisses achievement medals. Disputable, but probably worth it for consistency (if overlays aren't dismissed, neither should the medals)
osu.Game.Rulesets.Edit.SelectionBlueprint No? Selects objects in editor. OnMouseDown doesn't check button when dragging beatmap element, so it would be inconsistent if OnClick did for simple selection.
osu.Game.EditorMenuBar Yes Presses menu items
osu.Game.Screens.Edit.Components.RadioButtons.DrawableRadioButton Yes Selects radio button
osu.Game.Screens.Edit.Compose.Components.BeatDivisorControl.TickSliderBar Yes Seeks to mouse position on tick slider bar
osu.Game.Screens.Edit.Compose.Components.BlueprintContainer No? Deselects all objects in editor. If we allow to select with right click, then we should also allow to deselect with right click
osu.Game.Screens.Menu.Button Yes Presses button
osu.Game.Screens.Menu.OsuLogo Yes Presses logo button
osu.Game.Screens.Multi.Components.DisableableTabControl<T>.DisableableTabItem No Class is abstract, users can (and should) enforce their desired behaviour
osu.Game.Screens.Multi.Match.Components.MatchTabControl.TabItem Yes Selects tab item
osu.Game.Screens.Play.SkipOverlay.Button No Presses the "Skip" button at start of beatmap. Since RMB is the default for B2 on mouse, this should probably be allowed
osu.Game.Screens.Select.Carousel.DrawableCarouselBeatmap Yes Selects beatmap or starts play if selected already.
osu.Game.Screens.Select.Carousel.DrawableCarouselItem Yes Selects beatmap set.
osu.Game.Screens.Select.Options.BeatmapOptionsButton Yes Presses button in beatmap options wedge.
osu.Game.Screens.Select.Footer No Swallows events.
osu.Game.Screens.Select.FooterButton Yes Presses button in settings footer.
osu.Game.Users.Drawables.DrawableAvatar.ClickableArea Yes Clicks profile picture and opens profile overlay.
osu.Game.Rulesets.Osu.Edit.Blueprints.HitCircles.HitCirclePlacementBlueprint No? Places hit circle in editor. Should probably be consistent with dragging/selection?
osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.SliderPlacementBlueprint No? Places slider in editor. See above.
osu.Game.Rulesets.Osu.Edit.Blueprints.Spinners.SpinnerPlacementBlueprint No? Places spinner in editor. See above.
  • Test usage in osu.Game.Tests.Visual.Editor.TestSceneEditorComposeTimeline.StartStopButton omitted.
  • Tournament uses also omitted, as TournamentGameBase.TournamentInputManager.RightMouseManager has EnableClick => true.

Handle(UIEvent) overrides - status of guards for ClickEvent

Usage Guard suggested? Rationale
osu.Game.Graphics.UserInterface.ProcessingOverlay No Handler swallows any and all events
osu.Game.Input.IdleTracker No ClickEvent not handled explicitly, deferred to base
osu.Game.Rulesets.Edit.PlacementBlueprint No MouseEvent catch-all present. I tried breakpointing to see if any ClickEvents even arrive, never got a hit.
osu.Game.Rulesets.UI.RulesetInputManager<T> No ClickEvent not handled explicitly, deferred to base
osu.Game.Screens.Play.KeyCounterDisplay.Receptor No ClickEvent not handled explicitly, deferred to base
osu.Game.Rulesets.Osu.OsuInputManager No ClickEvent not handled explicitly, deferred to base
osu.Game.Rulesets.Osu.OsuInputManager.OsuKeyBindingContainer No ClickEvent not handled explicitly, deferred to base

OnMouseUp() overrides to potentially migrate to OnClick()

Usage Needs migrating? Handler effect
osu.Game.Graphics.Containers.OsuScrollContainer.OsuScrollBar No Reverts scrollbar colour to white upon left mouse release
osu.Game.Graphics.Cursor.MenuCursor No Returns to normal cursor size, undoes rotation on left mouse release
osu.Game.Graphics.UserInterface.OsuAnimatedButton No Returns to normal button size upon release
osu.Game.Graphics.UserInterface.OsuButton No Returns to normal button size upon release
osu.Game.Graphics.UserInterface.OsuSliderBar<T> No Reverts scrollbar nub colour to normal upon mouse release
osu.Game.Online.Leaderboards.RetrievalFailurePlaceholder.RetryButton No Reverts retry button to normal size upon mouse release
osu.Game.Overlays.Chat.Tabs.ChannelTabItem Yes? Has hacky click handling through OnMouseUp()... but for middle mouse
osu.Game.Overlays.Chat.Tabs.TabCloseButton No Returns to normal button size upon release
osu.Game.Overlays.KeyBinding.KeyBindingRow No Keybind input handling logic
osu.Game.Overlays.KeyBinding.KeyBindingRow.ClearButton No Stops finalisation and deselection of row
osu.Game.Overlays.Mods.ModButton Yes The cycling logic can be moved to OnClick() after right clicks are enabled
osu.Game.Overlays.Music.PlaylistItem No Makes playlist item non-draggable
osu.Game.Overlays.Settings.SettingsItem<T>.RestoreDefaultValueButton No Swallows events
osu.Game.Overlays.SettingsSubPanel.BackButton No Restores size of button
osu.Game.Screens.Edit.Compose.Components.Timeline.Timeline No Handles dragging
osu.Game.Screens.Edit.Compose.Components.BeatDivisorControl.TickSliderBar No Handles active state of slider bar
osu.Game.Screens.Menu.Button No Restores normal color
osu.Game.Screens.Menu.OsuLogo No Restores size of logo
osu.Game.Screens.Play.HUD.HoldForMenuButton.Button No Implements long press
osu.Game.Screens.Play.GameplayMenuOverlay No Swallows events
osu.Game.Screens.Play.KeyCounterMouse No Implements key counter for mouse
osu.Game.Screens.Play.SkipOverlay.FadeContainer No Re-displays skip button on mouse up
osu.Game.Screens.Play.SkipOverlay.Button No Restores normal size
osu.Game.Screens.Select.Options.BeatmapOptionsButton No Fades button flash
osu.Game.Screens.Select.FooterButton No Fades out button
osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.SliderPlacementBlueprint Yes Handles curve ending, should be moved to OnClick()
osu.Game.Rulesets.Mania.Edit.Blueprints.ManiaPlacementBlueprint<T> No Ends drag placement of object

Handle(UIEvent) overrides - potential migrations to ClickEvent

Usage Needs migrating? Rationale
osu.Game.Graphics.UserInterface.ProcessingOverlay No Handler swallows any and all events
osu.Game.Input.IdleTracker No Only saves last interaction time
osu.Game.Rulesets.Edit.PlacementBlueprint No Catch-all for MouseEvent
osu.Game.Rulesets.UI.RulesetInputManager<T> No Short and long press special handling
osu.Game.Screens.Play.KeyCounterDisplay.Receptor No Handles key counter flashes, clicks not applicable
osu.Game.Rulesets.Osu.OsuInputManager No Only MouseMoveEvent handled explicitly
osu.Game.Rulesets.Osu.OsuInputManager.OsuKeyBindingContainer No Catch-all, only checks AllowUserPresses flag

UX inconsistencies discovered

  • Sliders can be moved using right mouse.
  • OsuLogo doesn't shrink on right mouse down and doesn't grow back on right mouse up, all of the other buttons do.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment