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

Add amplitude argument to Input.vibrate_handheld #91143

Merged
merged 1 commit into from
May 2, 2024

Conversation

RadiantUwU
Copy link
Contributor

@RadiantUwU RadiantUwU commented Apr 25, 2024

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

You also need to register compatibility methods, I'll write a diff for you later today if needed, you can see this as a reference for how to do it:

core/input/input.cpp Outdated Show resolved Hide resolved
core/input/input.h Outdated Show resolved Hide resolved
core/os/os.h Outdated Show resolved Hide resolved
platform/android/java_godot_wrapper.cpp Outdated Show resolved Hide resolved
platform/android/java_godot_wrapper.cpp Outdated Show resolved Hide resolved
platform/ios/os_ios.mm Outdated Show resolved Hide resolved
platform/web/os_web.h Outdated Show resolved Hide resolved
platform/web/os_web.cpp Outdated Show resolved Hide resolved
platform/android/java_godot_wrapper.cpp Outdated Show resolved Hide resolved
platform/web/os_web.cpp Outdated Show resolved Hide resolved
@RadiantUwU
Copy link
Contributor Author

RadiantUwU commented Apr 25, 2024

You also need to register compatibility methods, I'll write a diff for you later today if needed, you can see this as a reference for how to do it:

Is it required if it just adds another argument with a default value?

It will continue to do the same behavior as previously, but now it has another argument that can be tweaked if need be

@AThousandShips
Copy link
Member

Is it required if it just adds another argument with a default value?

Yes, it's about binary compatibility, not general compatibility being broken, check the PR linked and this:

@AThousandShips
Copy link
Member

Please apply this diff, it provides the correct way to add compatibility (as seen in the PR linked above):

diff --git a/core/input/input.compat.inc b/core/input/input.compat.inc
new file mode 100644
index 0000000000..46df255607
--- /dev/null
+++ b/core/input/input.compat.inc
@@ -0,0 +1,41 @@
+/**************************************************************************/
+/*  input.compat.inc                                                      */
+/**************************************************************************/
+/*                         This file is part of:                          */
+/*                             GODOT ENGINE                               */
+/*                        https://godotengine.org                         */
+/**************************************************************************/
+/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
+/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur.                  */
+/*                                                                        */
+/* Permission is hereby granted, free of charge, to any person obtaining  */
+/* a copy of this software and associated documentation files (the        */
+/* "Software"), to deal in the Software without restriction, including    */
+/* without limitation the rights to use, copy, modify, merge, publish,    */
+/* distribute, sublicense, and/or sell copies of the Software, and to     */
+/* permit persons to whom the Software is furnished to do so, subject to  */
+/* the following conditions:                                              */
+/*                                                                        */
+/* The above copyright notice and this permission notice shall be         */
+/* included in all copies or substantial portions of the Software.        */
+/*                                                                        */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,        */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF     */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY   */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,   */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE      */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                 */
+/**************************************************************************/
+
+#ifndef DISABLE_DEPRECATED
+
+void Input::_vibrate_handheld_bind_compat_91143(int p_duration_ms = 500) {
+       vibrate_handheld(p_duration_ms, 1.0);
+}
+
+void Input::_bind_compatibility_methods() {
+       ClassDB::bind_compatibility_method(D_METHOD("vibrate_handheld", "duration_ms"), &Input::_vibrate_handheld_bind_compat_91143, DEFVAL(500));
+}
+
+#endif // DISABLE_DEPRECATED
diff --git a/core/input/input.cpp b/core/input/input.cpp
index 6a4efd555c..fa5a1881d4 100644
--- a/core/input/input.cpp
+++ b/core/input/input.cpp
@@ -29,6 +29,7 @@
 /**************************************************************************/

 #include "input.h"
+#include "input.compat.inc"

 #include "core/config/project_settings.h"
 #include "core/input/default_controller_mappings.h"
@@ -120,8 +121,7 @@ void Input::_bind_methods() {
        ClassDB::bind_method(D_METHOD("get_joy_vibration_duration", "device"), &Input::get_joy_vibration_duration);
        ClassDB::bind_method(D_METHOD("start_joy_vibration", "device", "weak_magnitude", "strong_magnitude", "duration"), &Input::start_joy_vibration, DEFVAL(0));
        ClassDB::bind_method(D_METHOD("stop_joy_vibration", "device"), &Input::stop_joy_vibration);
-       ClassDB::bind_method(D_METHOD("vibrate_handheld", "duration_ms"), &Input::vibrate_handheld, DEFVAL(500));
-       ClassDB::bind_compatibility_method(D_METHOD("vibrate_handheld", "duration_ms", "amplitude"), &Input::vibrate_handheld_amp, DEFVAL(500), DEFVAL(1.0));
+       ClassDB::bind_method(D_METHOD("vibrate_handheld", "duration_ms", "amplitude"), &Input::vibrate_handheld, DEFVAL(500), DEFVAL(1.0));
        ClassDB::bind_method(D_METHOD("get_gravity"), &Input::get_gravity);
        ClassDB::bind_method(D_METHOD("get_accelerometer"), &Input::get_accelerometer);
        ClassDB::bind_method(D_METHOD("get_magnetometer"), &Input::get_magnetometer);
@@ -804,14 +804,10 @@ void Input::stop_joy_vibration(int p_device) {
        joy_vibration[p_device] = vibration;
 }

-void Input::vibrate_handheld_amp(int p_duration_ms, float p_amplitude) {
+void Input::vibrate_handheld(int p_duration_ms, float p_amplitude) {
        OS::get_singleton()->vibrate_handheld(p_duration_ms, p_amplitude);
 }

-void Input::vibrate_handheld(int p_duration_ms) {
-       OS::get_singleton()->vibrate_handheld(p_duration_ms, 1.0);
-}
-
 void Input::set_gravity(const Vector3 &p_gravity) {
        _THREAD_SAFE_METHOD_

diff --git a/core/input/input.h b/core/input/input.h
index 43a8fee625..08b90645f0 100644
--- a/core/input/input.h
+++ b/core/input/input.h
@@ -267,6 +267,11 @@ private:
 protected:
        static void _bind_methods();

+#ifndef DISABLE_DEPRECATED
+       void _vibrate_handheld_bind_compat_91143(int p_duration_ms = 500);
+       static void _bind_compatibility_methods();
+#endif // DISABLE_DEPRECATED
+
 public:
        void set_mouse_mode(MouseMode p_mode);
        MouseMode get_mouse_mode() const;
@@ -323,8 +328,7 @@ public:

        void start_joy_vibration(int p_device, float p_weak_magnitude, float p_strong_magnitude, float p_duration = 0);
        void stop_joy_vibration(int p_device);
-       void vibrate_handheld(int p_duration_ms = 500);
-       void vibrate_handheld_amp(int p_duration_ms = 500, float amplitude = 1.0);
+       void vibrate_handheld(int p_duration_ms = 500, float p_amplitude = 1.0);

        void set_mouse_position(const Point2 &p_posf);

@RadiantUwU
Copy link
Contributor Author

Please apply this diff, it provides the correct way to add compatibility (as seen in the PR linked above)

image

@AThousandShips
Copy link
Member

Strange, it should work as it's extracted directly from this PR, try applying it manually

@RadiantUwU RadiantUwU force-pushed the add-input-amplitude branch 2 times, most recently from 79582c1 to ccc5d32 Compare April 26, 2024 14:01
@RadiantUwU RadiantUwU force-pushed the add-input-amplitude branch 2 times, most recently from ebf5cf5 to a4ed205 Compare April 26, 2024 14:05
@RadiantUwU
Copy link
Contributor Author

Fuck

@RadiantUwU RadiantUwU reopened this Apr 26, 2024
@RadiantUwU RadiantUwU force-pushed the add-input-amplitude branch 3 times, most recently from b15d71e to fac54c9 Compare April 26, 2024 15:35
@RadiantUwU
Copy link
Contributor Author

Technically my first time ever playing around with iOS documentation, i absolutely hate it 😊

@skysphr
Copy link
Contributor

skysphr commented Apr 26, 2024

Thank you for picking up on my proposal, I absolutely didn't expect someone else to implement it, and so quickly too!

@RadiantUwU
Copy link
Contributor Author

Can this be moved to the 4.4 milestone?

core/input/input.compat.inc Outdated Show resolved Hide resolved
platform/android/java_godot_wrapper.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Looks good!

@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 30, 2024
@RadiantUwU
Copy link
Contributor Author

Looks good! Could you squash the commits? See PR workflow for instructions.

I'll definitely try to but it says there's 130 commits in VSC 😭

@RadiantUwU
Copy link
Contributor Author

Looks good! Could you squash the commits? See PR workflow for instructions.

Done!

@akien-mga akien-mga changed the title Implement amplitude to Input.vibrate_handheld Implement amplitude to Input.vibrate_handheld May 1, 2024
platform/ios/os_ios.mm Outdated Show resolved Hide resolved
platform/web/os_web.cpp Outdated Show resolved Hide resolved
@RadiantUwU RadiantUwU requested a review from akien-mga May 2, 2024 13:15
@AThousandShips AThousandShips changed the title Implement amplitude to Input.vibrate_handheld Add amplitude argument to Input.vibrate_handheld May 2, 2024
@akien-mga
Copy link
Member

Looks good to me. Needs a rebase to solve a merge conflict.

Co-authored-by: A Thousand Ships <[email protected]>
Co-authored-by: m4gr3d <[email protected]>
@akien-mga akien-mga merged commit 5ea3f0b into godotengine:master May 2, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@RadiantUwU RadiantUwU deleted the add-input-amplitude branch June 3, 2024 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement amplitude parameter to Input.vibrate_handheld
6 participants