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

Tweak default crash handler message in exported projects #61908

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Jun 10, 2022

Follow-up to #61906.

When an exported project crashes, the crash handler message shouldn't reference the Godot issue tracker, as not all crashes are Godot's fault.

Reporting crashes that only occur on exported projects is still allowed, but it should not be done by people who aren't working on the project in question. This is especially true if we make crash logs more accessible, such as with #61906.

When an exported project crashes, the crash handler message
shouldn't reference the Godot issue tracker, as not all crashes
are Godot's fault.

Reporting crashes that only occur on exported projects is still allowed,
but it should not be done by people who aren't working on the project
in question.
@Calinou Calinou requested review from a team as code owners June 10, 2022 16:36
@Calinou Calinou added enhancement topic:core cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jun 10, 2022
@Calinou Calinou added this to the 4.0 milestone Jun 10, 2022
@akien-mga akien-mga merged commit 941575b into godotengine:master Jun 16, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jun 16, 2022
@Calinou Calinou deleted the crash-handler-message-tweak-exported-project branch June 16, 2022 14:41
@akien-mga
Copy link
Member

As you saw in #63839, this doesn't work as expected when the game crashes. The editor feature is only defined for the editor process, not for running the game from the editor / from the command line with a tools build.

@Calinou
Copy link
Member Author

Calinou commented Aug 3, 2022

The editor feature is only defined for the editor process

I'm pretty sure the editor feature is supposed to be true for any editor build (that's what the documentation states too). Otherwise, Engine::is_editor_hint() should be used.

@akien-mga
Copy link
Member

The editor feature is only defined for the editor process

I'm pretty sure the editor feature is supposed to be true for any editor build (that's what the documentation states too). Otherwise, Engine::is_editor_hint() should be used.

You're right:

#ifdef TOOLS_ENABLED
        if (p_feature == "editor") {
                return true;
        }
#else
        if (p_feature == "standalone") {
                return true;
        }
#endif

@Calinou
Copy link
Member Author

Calinou commented Aug 3, 2022

My guess is that the feature tag can't be read by the crash handler. As a workaround, I was thinking we should change the default value of the setting using #ifdef TOOLS_ENABLED instead of relying on the editor feature tag to override the default.

We could also use the generic crash handler message in editor builds. I think people will figure out that they can report editor crash backtraces 🙂

@RedMser
Copy link
Contributor

RedMser commented Aug 6, 2022

I encountered this problem in #64014 as well. Seems like the project settings' feature overrides are intentionally disabled in the editor.

@akien-mga
Copy link
Member

I encountered this problem in #64014 as well. Seems like the project settings' feature overrides are intentionally disabled in the editor.

I opened this issue to keep track of this: #64100.

reduz added a commit to reduz/godot that referenced this pull request Jan 13, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
Streq pushed a commit to Streq/godot that referenced this pull request Feb 9, 2023
* Overrides no longer happen for set/get.
* They must be checked with a new function: `ProjectSettings::get_setting_with_override()`.
* GLOBAL_DEF/GLOBAL_GET updated to use this

This change solves many problems:
* General confusion about getting the actual or overriden setting.
* Feature tags available after settings are loaded were being ignored, they are now considered.
* Hacks required for the Project Settings editor to work.

Fixes godotengine#64100. Fixes godotengine#64014. Fixes godotengine#61908.
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.

3 participants