Skip to content

Fix #2111: Add nullptr check before using the order object#2127

Merged
AaronVanGeffen merged 3 commits intoOpenLoco:masterfrom
Mohsin-Ul-Islam:fix-vehicle-view
Nov 23, 2023
Merged

Fix #2111: Add nullptr check before using the order object#2127
AaronVanGeffen merged 3 commits intoOpenLoco:masterfrom
Mohsin-Ul-Islam:fix-vehicle-view

Conversation

@Mohsin-Ul-Islam
Copy link
Copy Markdown
Contributor

@Mohsin-Ul-Islam Mohsin-Ul-Islam commented Sep 29, 2023

Fixes #2111

auto orderRing = Vehicles::OrderRingView(orderFrame.orderOffset);
auto* order = orderRing.atIndex(0);
if (!order->hasFlags(Vehicles::OrderFlags::HasNumber))
if (!order || !order->hasFlags(Vehicles::OrderFlags::HasNumber))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this hides the issue rather than addressing it. Why is order a nullptr in the first place? The code assumes that there is an order to display, so why isn't there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi, yes I understand the problem a bit better now. I am still figuring out the issue.

Thank you

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Although it does hide the issue i tried to find the source before and wasn't successful. So if you can't work it out it doesn't matter we can just merge this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggest we add a comment that we don't know why we can get into a bad state here but if we do then this makes it safe.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found the source of the issue. I'll keep this PR but augment it with an additional fix.

@duncanspumpkin duncanspumpkin added this to the v23.10+ milestone Nov 6, 2023
@AaronVanGeffen AaronVanGeffen changed the title fix(viewport): add nullptr check before using the order object Fix #2111: Add nullptr check before using the order object Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in Order Flag viewport draw

3 participants