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

AnimatedSprite2d: SpriteFrames Editor #76729

Closed
almusx opened this issue May 4, 2023 · 19 comments · Fixed by #77804
Closed

AnimatedSprite2d: SpriteFrames Editor #76729

almusx opened this issue May 4, 2023 · 19 comments · Fixed by #77804

Comments

@almusx
Copy link

almusx commented May 4, 2023

Godot version

v4.1.dev1.official [db13026]

System information

Windows 10, Compatibility Rendering Driver, GTX 1660 Ti (531.79)

Issue description

The mouse cursor disappears when i'm clicking and holding the mouse button on the little arrows in the Frame Duration option and moving the mouse over the Edit Box (with the x1 text).

image

Steps to reproduce

  1. Add AnimatedSprite2d node
  2. Load spritesheet
  3. Open SpriteFrames editor
  4. Select a frame
  5. click and hold the mouse button over the little up or down arrow on the Frame Duration Option
  6. While holding the mouse button move left over the edit box.

Minimal reproduction project

FrameDurationTest.zip

@AThousandShips
Copy link
Member

AThousandShips commented May 4, 2023

Does this happen in other cases where that type of input box is used? The one with the little arrows? Like the FPS one to the left

@almusx
Copy link
Author

almusx commented May 4, 2023

Does this happen in other cases where that type of input box is used? The one with the little arrows? Like the FPS one to the left

changing the fps works as expected, clicking the little arrows works and holding the mouse button i can change the fps moving mouse up /down or left / right. The mouse cursor disapear while holding and became visible after releasing.

I tried again the frame duration arrows and holding the mouse button, the mouse cursor disapear while moving the mouse left/right, dont need to move over the edit box and the mouse is locked on the center of the screen, if i right click its shows "add node here"
"instantiate scene here"

@AThousandShips
Copy link
Member

Al in what way is it working differently?

@almusx
Copy link
Author

almusx commented May 4, 2023

Al in what way is it working differently?

after releasing the mouse button, the mouse still not visible and locked on the center of the screen, i cant do anything, only alt+f4 to exit.

edit: i can use keyboard shortcuts to save scene, open menus, etc, i think the mouse is only not visible.

@almusx
Copy link
Author

almusx commented May 4, 2023

i tested again, if i Alt+tab or press the Windows Key, the mouse become visible but after i click on the godot it disapear again and get locked to the screen center, also holding the mouse button while clicking the arrows and moving the mouse doesnt change the frame duration like the fps.

@dalexeev
Copy link
Member

dalexeev commented May 4, 2023

Duplicate of #75652 (was closed).

@AThousandShips
Copy link
Member

AThousandShips commented May 4, 2023

It does capture the mouse which makes it invisible and centered, sounds like it might be failing to restore it properly, it should be cancelled when exiting the window thoug which it seems doesn't happen here

@AThousandShips

This comment was marked as outdated.

@kleonc
Copy link
Member

kleonc commented May 4, 2023

For reference, this is where it is captured:

@AThousandShips Not really, that's a SpinBox:

frame_duration = memnew(SpinBox);
frame_duration->set_prefix(String::utf8("×"));
frame_duration->set_min(SPRITE_FRAME_MINIMUM_DURATION); // Avoid zero div.
frame_duration->set_max(10);
frame_duration->set_step(0.01);
frame_duration->set_custom_arrow_step(0.1);
frame_duration->set_allow_lesser(false);
frame_duration->set_allow_greater(true);
hbc->add_child(frame_duration);

frame_duration->connect("value_changed", callable_mp(this, &SpriteFramesEditor::_frame_duration_changed));

Seems like after capturing the mouse on drag start:

if (mm.is_valid() && (mm->get_button_mask().has_flag(MouseButtonMask::LEFT))) {
if (drag.enabled) {
drag.diff_y += mm->get_relative().y;
double diff_y = -0.01 * Math::pow(ABS(drag.diff_y), 1.8) * SIGN(drag.diff_y);
set_value(CLAMP(drag.base_val + step * diff_y, get_min(), get_max()));
} else if (drag.allowed && drag.capture_pos.distance_to(mm->get_position()) > 2) {
Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_CAPTURED);
drag.enabled = true;
drag.base_val = get_value();
drag.diff_y = 0;
}
}

it ends up not being released, for whatever reason this is not executed in this case:
if (mb.is_valid() && !mb->is_pressed() && mb->get_button_index() == MouseButton::LEFT) {
//set_default_cursor_shape(CURSOR_ARROW);
range_click_timer->stop();
_release_mouse();
drag.allowed = false;
line_edit->clear_pending_select_all_on_focus();
}

void SpinBox::_release_mouse() {
if (drag.enabled) {
drag.enabled = false;
Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_HIDDEN);
warp_mouse(drag.capture_pos);
Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_VISIBLE);
}
}


What's strange is it works just fine for the FPS SpinBox from the same plugin, which is setuped seemingly the same way:

anim_speed = memnew(SpinBox);
anim_speed->set_suffix(TTR("FPS"));
anim_speed->set_min(0);
anim_speed->set_max(120);
anim_speed->set_step(0.01);
anim_speed->set_custom_arrow_step(1);
anim_speed->set_tooltip_text(TTR("Animation Speed"));
anim_speed->connect("value_changed", callable_mp(this, &SpriteFramesEditor::_animation_speed_changed));
hbc_animlist->add_child(anim_speed);

@AThousandShips
Copy link
Member

Oh damn my bad, that is strange

@korypostma
Copy link
Contributor

korypostma commented May 25, 2023

Yeah I replicated this bug the first time I tried, now hopefully from within the debugger...

Edit: followed it through the debugger and I cannot replicate it, so weird... Will try more later, this could be quite annoying though.

@korypostma
Copy link
Contributor

korypostma commented May 25, 2023

I'm able to replicate this in a debug build as long as I attach the debugger after starting, I was not able to replicate it from debugging within VS. From what I can tell it seems that the SpinBox loses focus and the mouse pointer because moving the mouse while holding down the left mouse button no longer modifies the value and it appears like the main viewport took over control / focus but then the input system is left in a limbo state and the only way to get my mouse cursor back was to force the issue within VS's intermediate window.

Do you guys have any ideas on how best to test this to really see what's going on or if the viewport is stealing focus away from the SpinBox?

Edit: to check the status of things I added a bunch of print_line calls and to get the mouse cursor back I have to set a breakpoint on the Input::get_mouse_mode() function and then use this in the Intermediate window to get the mouse cursor back:
Input::get_singleton()->set_mouse_mode(Input::MouseMode::MOUSE_MODE_VISIBLE)

@korypostma
Copy link
Contributor

This appears to be a combination of things.

  1. CanvasItemEditorViewport is grabbing focus from the SpinBox while MOUSE_CAPTURED mode is enabled.
  2. The LineEdit control within the SpinBox is the one receiving and responding to messages, so SpinBox never receives messages when it loses focus, etc., hence it is unable to call _release_mouse when it normally should be able to.

I'm still digging into this, but yeah, this is not that easy of a fix.

@korypostma
Copy link
Contributor

Well, I need to do more testing, but if I remove the call to hide(); in SpriteFramesEditor::edit(...) then the control acts like it should. The call to hide this control was resulting in the LineEdit to lose focus, hence allowing the Viewport and others to take focus, and notifications were not always acted upon.

image

@korypostma
Copy link
Contributor

Checking if the SpinBox LineEdit's are in focus, if the mouse is captured, if either SpinBox is in dragging mode all seems to prevent this issue. I think the only one we really need to check though is if either are in dragging mode (I created this function to access this in SpinBox though).

void SpriteFramesEditor::edit(Ref<SpriteFrames> p_frames) {
	_update_stop_icon();

	Control *focus = get_viewport()->gui_get_focus_owner();
	const bool is_focus_on_text = (focus != nullptr && ((focus == frame_duration->get_line_edit()) || (focus == anim_speed->get_line_edit())));
	const bool is_mouse_captured = Input::get_singleton()->get_mouse_mode() == Input::MOUSE_MODE_CAPTURED;
	const bool is_dragging = frame_duration->is_dragging() || anim_speed->is_dragging();
	print_line("is_focus_on_text=", is_focus_on_text, " is_mouse_captured=", is_mouse_captured, " is_dragging=", is_dragging);

	if (!p_frames.is_valid()) {
		if (is_focus_on_text || is_mouse_captured || is_dragging) {
			return;
		}

		frames.unref();
		hide();
		return;
	}

@korypostma
Copy link
Contributor

I looked into this again and long story short, it is actually only the is_focus_on_text variable in the last comment that matters. The other two checks it will still occur, but if is_focus_on_text is true then it will never happen. I'll modify my source code and provide a pull-request linking this issue and possibly the older one. I think there is a lot of work to be done on the GUI side of the house though, this could all be handled so much better (most notably the MOUSE_CAPTURE stuff).

@korypostma
Copy link
Contributor

korypostma commented Jun 2, 2023

Another trip through this issue and this is what I have discovered...

Upon creation an ObjectID is assigned to the plugin with this value: SpriteFramesEditorPlugin _instance_id.id: 912798038723

But in EditorNode::hide_unused_editors(...) it is checking kv.key is an instance in ObjectDB and that value is as follows: kv.key = 1674684984504.

Since these keys do not match, it tries to hide the plugin and when the plugin is captured it is then losing the mouse cursor, focus, and other things.

Edit: It seems these keys don't match because the active_plugins store the owner ObjectID, not the plugins. Anyhow, anyone else have any ideas on what is going on here?

@ajreckof
Copy link
Member

ajreckof commented Jun 3, 2023

So after taking a look at it with @korypostma, this is what we have found, there is in fact two bug that results in what we see here :

  • First the issue I created just above which is a SpinBox bug that when it is hidden it does not release the mouse which in turn keep the mouse invisible eternally. This is what cause the mouse to stay invisible here.
  • Second here more than just two actions happening at the same time because of bad luck it is dragging in itself that triggers the resource to be changed which triggers the update of the whole inspector by AnimatedSprite2d plugin editor. which in turn destroys the EditorPropertyResource editing the SpriteFrames. Therefore the SpriteFrames bottom editor is being hidden.

@dalexeev
Copy link
Member

There is a small chance that #79692 fixes this bug. It doesn't reproduced on my configuration, so I'd appreciate it if you could test with the PR applied.

@YuriSizov YuriSizov added this to the 4.2 milestone Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants