Skip to content
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

Enhanced brush mode #17

Merged
merged 10 commits into from
Feb 22, 2025

Conversation

zacharied
Copy link
Contributor

@zacharied zacharied commented Oct 20, 2024

TODO

[x] Refactor selection area code
[x] Enable click selection with FilterSubmode
[x] Fix missing Brush button text
[x] Fix crash when clicking on LaneBlock in batch mode
[x] Add menu bar buttons so users can discover keybinds

Brush mode improvements

Brush mode has been changed. When in Brush Mode, an overlay is shown in the bottom left. It shows the input object.

OngekiFumenEditor_vEkEa1HRjO

By default, the input object is "Clipboard", which is the same as the old brush mode behavior.

Changing input object

The user can press keys to change the input object. See CommandDefinitions for the full list.

OngekiFumenEditor_xtONGUNjXN

Insert input object

Left Click inserts the input object. If Alt or Ctrl is held, modifiers are applied to some inputs. If Shift is held, the previous selection will be kept; otherwise the selection will be cleared.

OngekiFumenEditor_RF3MrJ9cxg

Deleting input object

If the input object is not "Clipboard", Right Click deletes instances of the input object.

OngekiFumenEditor_Hphen1ggba

Change selection

If Alt is held while clicking and dragging, normal box selection will be used. This is so that the user does not have to exit Brush Mode to change selection.

OngekiFumenEditor_9bxDUu186U

Questions

@MikiraSora, before I finish localizing and bugfixing, I want to make sure you like this.

  • Are you okay with implementing brush mode as a Behavior?
  • Any other keybinds I should add to brush mode?
  • I wanted to add a "preview" of the input object below the mouse cursor. What would be the best way to do that?

@zacharied zacharied marked this pull request as draft October 20, 2024 18:42
@MikiraSora
Copy link
Collaborator

MikiraSora commented Oct 21, 2024

It shouldn't continue to be called BrushMode, because there are too many operations to memorize.
Maybe we can just combine all these functions, including BrushMode, into a new name, BatchMode.
Then the original mode could be called NormalMode, so that new users can quickly get started in this mode, and users who need more operations can use BatchMode.

Are you okay with implementing brush mode as a Behavior?

I haven't delved much into the full functionality of WPF, so this implementation requires me to understand it before I can draw conclusions

Any other keybinds I should add to brush mode?

None at the moment, but I would suggest that this needs to be compatible with the Keybinding feature, as users may need to modify the shortcuts to make it easier for them to use them.

I wanted to add a "preview" of the input object below the mouse cursor. What would be the best way to do that?

I do have an idea, is that the user's current pointer operation can be displayed near the pointer, for example, in your demonstration of the function #Deleting input object, when the user presses and holds the pointer and drags it, the words “Deletion of objects within the range” will be displayed near the pointer to indicate that
I'll try to make a prototype of this feature when I get around to it.

Anyway, it's not recommended to merge this PR yet, need to wait for me to finish the keybinding feature first, sorry, but it's coming soon!

[Key.D4] = new BrushModeInputWallRight(),
[Key.D5] = new BrushModeInputLaneColorful(),
[Key.T] = new BrushModeInputTap(),
[Key.D] = new BrushModeInputHold(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

The H key should be assigned here

@zacharied
Copy link
Contributor Author

I will rename "Brush Mode" to "Batch Mode" and add new keybindings called KBD_Batch_AddTap, KBD_Batch_AddHold, etc.

@zacharied
Copy link
Contributor Author

Batch Mode requires its own set of keybindings, separate from the normal ones. So I have started to make a "layer" system.

There are three layers, Global, Normal, and Batch. Keybindings in Normal are allowed to conflict with keybindings from Batch mode.

Global keybinds are always active.
Normal keybinds are detached when Batch mode is enabled.
Batch keybinds are attached when Batch mode is enabled.

For keybindings such as FastAddConnectableChild, I want to be accessible in Batch mode. So, I will make the Layers into a Flag, and each KeyBindingDefinition can have multiple layers.

@zacharied
Copy link
Contributor Author

zacharied commented Oct 24, 2024

I used the feature a lot and made some changes.

InputObjects are now referred to as "Submodes". There are two types of submodes. They determine what is done when clicking.

  • InputObjectSubmode: Clicking creates new objects
  • FilterObjectSubmode: Clicking selects objects like in Normal Mode, but only certain types are selected

A new icon is displayed when in a FilterObjectSubmode.

OngekiFumenEditor_djIHN0ZZdH

The filters are:

  • FloatingObject: selects bells, bullets, & flicks
  • DockableObject: selects taps & holds
  • Lane: selects lanes & walls

Documentation

Default keybinds

✏️: InputSubmode
🔎: FilterSubmode

Key Action
Tilde ✏️WallLeft
1 ✏️LaneLeft
2 ✏️LaneCenter
3 ✏️LaneRight
4 ✏️WallRight
5 ✏️LaneColorful
6 🔎Lanes
E ✏️Bell
T ✏️ Tap
H ✏️Hold
F ✏️Flick
Z ✏️LaneBlock
V ✏️Clipboard
Y 🔎Dockable Objects
U 🔎Floating Objects

These tables describe the behavior when clicking with different modifiers in Batch Mode.

Left click operations

Modifier InputObject FilterObject
None Lane, hold, LaneBlock: Place object and select
Tap, flick, bell, bullet: Place object
Drag to select filtered object
Alt Normal mode selection Normal mode selection
Shift Lanes & holds: Place object (keep current selection)
Flick & Lane block: Place object (change direction)
Others: same as None
Drag to select filtered object (keep current selection)
Ctrl Taps, holds, & flicks: Place object (critical) Same as None
Ctrl+Alt Drag to select only input object Same as None

Right click operations

Modifier InputObject FilterObject
None Clipboard: Nothing
Others: Drag to delete input object
Drag to delete filtered object
Alt Drag to delete all objects Drag to delete all objects
Shift Same as None Same as None
Ctrl Same as None Same as None
Ctrl+Alt Same as None Same as None

TODO

  • Some right click behavior is buggy
  • Finish renaming classes BrushModeBatchMode
  • Test

@zacharied zacharied marked this pull request as ready for review October 24, 2024 23:21
@zacharied
Copy link
Contributor Author

zacharied commented Oct 24, 2024

All planned functionality is complete.

For now, please refer to the table in the previous comment for documentation. We should add the table to the wiki.

If you have any ideas, let me know. I think there are improvements that could be made, but I am happy with the base functionality.

@MikiraSora
Copy link
Collaborator

  • no action to Clipboard place (and select)
  • LButton + InputObject + Ctrl+Alt seems same as LButton + InputObject + Alt when user want to dragging something

@MikiraSora
Copy link
Collaborator

MikiraSora commented Oct 25, 2024

image
The latter operation can be merged directly into the former, since it is only necessary to select and press C. Or the automatic selection after placement can also be done by pressing C

Therefore Ctrl and Ctrl+Alt combinations can be omitted altogether

@MikiraSora
Copy link
Collaborator

One more question.
In “InputObject” mode, clicking on a place to place an object and then clicking again on the same place creates a new object. It should be modified so that if there is an object of the same type near the place to be generated, then it can be selected instead. Because in the vast majority of cases there is no need to add multiple objects of the same type to the same place

@zacharied
Copy link
Contributor Author

zacharied commented Oct 25, 2024

Therefore Ctrl and Ctrl+Alt combinations can be omitted altogether

I think we will still need to keep Ctrl+Alt because it allows "Drag to select only input object".

As you mentioned it is currently not working but I will fix

@zacharied
Copy link
Contributor Author

zacharied commented Oct 25, 2024

I am too tired, I will finish this tomorrow 😴

@zacharied
Copy link
Contributor Author

Just kidding wwwwww

Need to bug test tomorrow.

@zacharied
Copy link
Contributor Author

oo i broke clipboard again one sec

@zacharied
Copy link
Contributor Author

Hmm, there is a bug where holding Shift + Alt in InputObject mode will always drag the selection. Sometimes it also breaks the undo/redo stack.

@zacharied
Copy link
Contributor Author

zacharied commented Oct 26, 2024

I've left the SelectRegion code in a pretty ugly state, but I plan on refactoring it to be less fragile.

I will do that in a separate PR.

@zacharied
Copy link
Contributor Author

i think it is done 🙏

I will use this build to work on my fumens for a few days to find more bugs.

@MikiraSora
Copy link
Collaborator

MikiraSora commented Oct 26, 2024

I will do that in a separate PR.

You can deal with them in this PR together

Some problems I had reported are still exist. such as clipboard and generate same object in same place

@zacharied
Copy link
Contributor Author

Placement of objects behavior is annoying since it ignores bullet palette when checking for objects at same position.

@zacharied
Copy link
Contributor Author

zacharied commented Oct 26, 2024

Added a method Clashes(OngekiObjectBase other). This is used to detect if an object is allowed to be placed on top of another.

@MikiraSora
Copy link
Collaborator

MikiraSora commented Oct 27, 2024

Clashes() should implement at editor because it's only for editor

use match pattern

@zacharied
Copy link
Contributor Author

zacharied commented Oct 27, 2024

Started refactor... sometimes there is the following crash while dragging the selection box around

----------Exception Catcher----------
Program notice a (unhandled) exception from object: System.Windows.Threading.Dispatcher(System.Windows.Threading.Dispatcher)

Exception lv.0 : Object reference not set to an instance of an object.
Stack :    at OngekiFumenEditor.Modules.FumenVisualEditor.ViewModels.FumenVisualEditorViewModel.OnMouseMove(Point pos)
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
----------------------------

I am too sleepy to figure out the cause tonight 😄

@zacharied zacharied marked this pull request as draft October 27, 2024 07:40
@zacharied
Copy link
Contributor Author

zacharied commented Oct 27, 2024

Fixed it! It was a race condition caused by setting it to null in the MouseUp in the middle of a MouseMove call.

@zacharied zacharied marked this pull request as ready for review October 27, 2024 19:37
@MikiraSora
Copy link
Collaborator

at fad1143 KeyboardAction_DeleteSelectingObjects()
Here I used a new implementation to fix the problem

QQ20241028-15741-HD.mp4

But I see that you're deleting objects elsewhere as well, so I'd suggest that you need to follow up on that as well

@zacharied
Copy link
Contributor Author

Thanks, I will follow your code for the other deletion functions

@zacharied
Copy link
Contributor Author

zacharied commented Oct 28, 2024

Could you merge fad1143 into this PR? So i don't have to duplicate your deletion code.

@MikiraSora
Copy link
Collaborator

Could you merge fad1143 into this PR? So i don't have to duplicate your deletion code.

how , I merged your current work and push to batch_mode in my repo

@zacharied
Copy link
Contributor Author

Oh, i can do it myself, sorry

@zacharied
Copy link
Contributor Author

Crash when clicking on LaneBlock in batch mode:

----------Exception Catcher----------
Program notice a (unhandled) exception from object: System.Windows.Threading.Dispatcher(System.Windows.Threading.Dispatcher)

Exception lv.0 : Unable to cast object of type 'OngekiFumenEditor.Base.OngekiObjects.LaneBlockArea' to type 'OngekiFumenEditor.Base.OngekiMovableObjectBase'.
Stack :    at OngekiFumenEditor.Modules.FumenVisualEditor.ViewModels.FumenVisualEditorViewModel.<>c__DisplayClass292_0.<GetConflictingObject>b__0(IDisplayableObject x)
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at OngekiFumenEditor.Modules.FumenVisualEditor.Behaviors.BatchMode.BatchModeBehavior.PerformBrush(BatchModeInputSubmode submode)
   at OngekiFumenEditor.Modules.FumenVisualEditor.Behaviors.BatchMode.BatchModeBehavior.MouseUp(MouseButtonEventArgs args)
   at Microsoft.Xaml.Behaviors.TriggerBase.InvokeActions(Object parameter) in D:\a\_work\1\s\src\Microsoft.Xaml.Behaviors\TriggerBase.cs:line 153
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
   at System.Windows.Input.InputManager.ProcessStagingArea()
   at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
   at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
----------------------------

@zacharied
Copy link
Contributor Author

zacharied commented Oct 29, 2024

I'm noticing this bug starting today, was it one of the recent commits?

Bottom green object is a TAP and top green object is a HOLD.

OngekiFumenEditor_XaJFbZp3ZQ.mp4

@MikiraSora
Copy link
Collaborator

I'm noticing this bug starting today, was it one of the recent commits?

Bottom green object is a TAP and top green object is a HOLD.

OngekiFumenEditor_XaJFbZp3ZQ.mp4

fixed and plz pull

@MikiraSora
Copy link
Collaborator

Crash when clicking on LaneBlock in batch mode:

----------Exception Catcher----------
Program notice a (unhandled) exception from object: System.Windows.Threading.Dispatcher(System.Windows.Threading.Dispatcher)

Exception lv.0 : Unable to cast object of type 'OngekiFumenEditor.Base.OngekiObjects.LaneBlockArea' to type 'OngekiFumenEditor.Base.OngekiMovableObjectBase'.
Stack :    at OngekiFumenEditor.Modules.FumenVisualEditor.ViewModels.FumenVisualEditorViewModel.<>c__DisplayClass292_0.<GetConflictingObject>b__0(IDisplayableObject x)
   at System.Linq.Enumerable.TryGetFirst[TSource](IEnumerable`1 source, Func`2 predicate, Boolean& found)
   at OngekiFumenEditor.Modules.FumenVisualEditor.Behaviors.BatchMode.BatchModeBehavior.PerformBrush(BatchModeInputSubmode submode)
   at OngekiFumenEditor.Modules.FumenVisualEditor.Behaviors.BatchMode.BatchModeBehavior.MouseUp(MouseButtonEventArgs args)
   at Microsoft.Xaml.Behaviors.TriggerBase.InvokeActions(Object parameter) in D:\a\_work\1\s\src\Microsoft.Xaml.Behaviors\TriggerBase.cs:line 153
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
   at System.Windows.Input.InputManager.ProcessStagingArea()
   at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
   at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
----------------------------

LaneBlockArea no X axis to move so it's actual not a OngekiMovableObjectBase
OngekiMovableObjectBase means its has X/Y axis to move.

@zacharied
Copy link
Contributor Author

LaneBlockArea no X axis to move so it's actual not a OngekiMovableObjectBase OngekiMovableObjectBase means its has X/Y axis to move.

Thank you, i should have mentioned i fixed it with db8b813

@zacharied
Copy link
Contributor Author

I added a new toolbar for users who do not know the keybindings.
image

@zacharied
Copy link
Contributor Author

Updated the icons to look nicer in dark mode.
image

@zacharied
Copy link
Contributor Author

I think I am done with this PR. I will use it to make some charts and find bugs.

@MikiraSora
Copy link
Collaborator

I think I am done with this PR. I will use it to make some charts and find bugs.

It will be merged after days if there is no problem

@MikiraSora
Copy link
Collaborator

It will be merged after hours witch I wake up if there is no problem
And I will take a time to update wiki and others

@MikiraSora MikiraSora merged commit ada7e56 into NyagekiFumenProject:master Feb 22, 2025
@MikiraSora
Copy link
Collaborator

Congratulations! This is a huge improve for program and users. Thx your PR❤️

@zacharied
Copy link
Contributor Author

Thank you! Please let me know if there is any feedback from Chinese users about this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants