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

Use TryGet instead of Get and assign a default fallback Color #16456

Merged
merged 1 commit into from May 4, 2019

Conversation

@LipkeGu
Copy link
Member

commented Apr 28, 2019

This PR prevents a crash in situations when custom mods was used and OpenRA is reset to its initial state (like deleting the Support Directory. I decided to set a fallback color to white for both Chat and System Messages to stay safe.

Fixes #16488.

Use TryGet instead of Get and assign a default Fallback Color
This Fixes a crash in Situations when custom mods are used and reset OpenRA to its initial state.
@pchote

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

These colors are set in the mod's chrome metrics, and have nothing to do with the support directory.
ChromeMetrics.TryGet should only be used in cases where we have good reason to expect a definition to intentionally be missing (e.g. the per-faction overrides for the color picker, which most mods don't use).

In this case it is the mods responsibility to update itself to be compatible with the engine changes. We have the SDK update notes to document these changes (which will be updated before we tag a new playtest).

@LipkeGu

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2019

The problem is that at the time no mod is loaded. In this situation OpenRA crashes right before it loads any mods. So when we start openRA by default with ts and delete the ts content, then this crash is happen. so it was in my case.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

It looks like #16355 forgot to update the modcontent mod's metrics.yaml.

@pchote

This comment has been minimized.

Copy link
Member

commented May 3, 2019

After investigating this in more detail with @teinarss we realized that this is going to be the best solution to the bug.

Fixes #16488.

@pchote
pchote approved these changes May 3, 2019

@pchote pchote added the PR: Needs +2 label May 3, 2019

@reaperrr reaperrr merged commit 7f7809f into OpenRA:bleed May 4, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.