Skip to content

Allow disabling joypad support on Windows#84848

Draft
jsjtxietian wants to merge 1 commit intogodotengine:masterfrom
jsjtxietian:add-support-for-disable-joypad
Draft

Allow disabling joypad support on Windows#84848
jsjtxietian wants to merge 1 commit intogodotengine:masterfrom
jsjtxietian:add-support-for-disable-joypad

Conversation

@jsjtxietian
Copy link
Contributor

@jsjtxietian jsjtxietian commented Nov 13, 2023

On my way implementing godotengine/godot-proposals#8391

@AThousandShips AThousandShips changed the title Allow to disable joypad support on windows Allow disabling joypad support on Windows Nov 13, 2023
@AThousandShips AThousandShips added this to the 4.x milestone Nov 13, 2023
@jsjtxietian jsjtxietian force-pushed the add-support-for-disable-joypad branch from 0446044 to 4576e68 Compare November 13, 2023 16:16
@jsjtxietian jsjtxietian marked this pull request as ready for review November 13, 2023 16:17
@jsjtxietian jsjtxietian requested review from a team as code owners November 13, 2023 16:17
@jsjtxietian jsjtxietian marked this pull request as draft November 14, 2023 02:12
@jsjtxietian jsjtxietian changed the title Allow disabling joypad support on Windows [WIP] Allow disabling joypad support on Windows Nov 16, 2023
@jsjtxietian jsjtxietian force-pushed the add-support-for-disable-joypad branch from 4576e68 to cf485c0 Compare November 17, 2023 15:06
@jsjtxietian jsjtxietian force-pushed the add-support-for-disable-joypad branch 2 times, most recently from 488e632 to 18e0210 Compare November 18, 2023 12:28
@jsjtxietian jsjtxietian force-pushed the add-support-for-disable-joypad branch from 18e0210 to 0c7a2f1 Compare November 24, 2023 15:36
@jsjtxietian jsjtxietian marked this pull request as ready for review November 24, 2023 15:36
@jsjtxietian jsjtxietian requested review from a team as code owners November 24, 2023 15:36
main/main.cpp Outdated
Copy link
Member

@AThousandShips AThousandShips Nov 24, 2023

Choose a reason for hiding this comment

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

I think this should be moved to where other editor settings are defined so that it can be documented, I suspect it not being available for documentation is because either it occurs in the wrong place, or the doctool generation doesn't set editor to true

Copy link
Contributor Author

@jsjtxietian jsjtxietian Nov 26, 2023

Choose a reason for hiding this comment

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

Yes, initially I was thinking about get this editor setting defined before the joypad is initialized, but it seems it's too early. And now we have to find a place to define it, maybe in Input?

Copy link
Member

@AThousandShips AThousandShips Nov 26, 2023

Choose a reason for hiding this comment

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

You need to remove editor to make it work, it is not assigned when running the doctool, and shouldn't matter anyway as you're already in the tool check

It also needs to be moved up before the doctool is run, so up to 2905

Copy link
Contributor Author

@jsjtxietian jsjtxietian Nov 26, 2023

Choose a reason for hiding this comment

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

Sorry I don't get it, I understand now that I need to remove the editor check, but what do you mean by "It also needs to be moved up before the doctool is run" ? Sorry I don't understand the doctool part !

Copy link
Member

Choose a reason for hiding this comment

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

The part that generates the documentation is tun on line 2905, hence why it needs to be moved up there 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks, but anyway I think I need to move it to a later stage because now the EditorSettings is not instantiated yet. How stupid am I, I did not check the console error message.

Now the problem is, the joypad is inited before the EditorSettings, so editor def and get will not work, which means under the current implentation, the joypad editor setting is useless ( User can still use a command line arg but that;'s more work ).

Copy link
Member

Choose a reason for hiding this comment

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

You should probably register it in the editor settings file, but might not work in any case as you say

project setting, command arg and windows support
@jsjtxietian jsjtxietian force-pushed the add-support-for-disable-joypad branch from 0c7a2f1 to 54f2f9a Compare November 26, 2023 14:19
@jsjtxietian jsjtxietian changed the title [WIP] Allow disabling joypad support on Windows Allow disabling joypad support on Windows Nov 27, 2023
@jsjtxietian jsjtxietian marked this pull request as draft November 27, 2023 02:46
@Nintorch
Copy link
Contributor

Might be superseded by #105513 , and now we don't have the joypad_windows.cpp/.h files anymore.

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.

5 participants

Comments