Skip to content

fix case where viewportIsFocusedOnEntity is called with no args#3122

Merged
duncanspumpkin merged 4 commits intoOpenLoco:masterfrom
LeftofZen:fixViewportFollowBugFrom3121
Jun 30, 2025
Merged

fix case where viewportIsFocusedOnEntity is called with no args#3122
duncanspumpkin merged 4 commits intoOpenLoco:masterfrom
LeftofZen:fixViewportFollowBugFrom3121

Conversation

@LeftofZen
Copy link
Copy Markdown
Contributor

@LeftofZen LeftofZen commented Jun 13, 2025

Fixes an edge case noted here #3121 (comment)

@LeftofZen LeftofZen requested a review from ZehMatt June 13, 2025 02:25
@LeftofZen LeftofZen added the bug-openloco A problem or bug in OpenLoco (but not vanilla) label Jun 13, 2025
Add ZehMatts suggestion

Co-authored-by: Matt <5415177+ZehMatt@users.noreply.github.com>
}

// If the argument provided is not EntityId::null check for a match.
if (targetEntity != EntityId::null && viewportConfigurations[0].viewportTargetSprite != targetEntity)
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.

I can't comment on unaffected lines, line 688 needs to be viewportConfigurations[0].viewportTargetSprite != EntityId::null to see if it follows any entity.

Copy link
Copy Markdown
Contributor Author

@LeftofZen LeftofZen Jun 26, 2025

Choose a reason for hiding this comment

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

hmm i had a look at this again and i still don't like how it reads, I think the way it's written makes it confusing and is probably why this keeps making bugs. I think I'll rewrite it to be explicily clear about what is happening, and remove the implicit parameter since that confuses things more

@duncanspumpkin duncanspumpkin added this to the v25.06 milestone Jun 30, 2025
@duncanspumpkin duncanspumpkin merged commit 43cfdf6 into OpenLoco:master Jun 30, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-openloco A problem or bug in OpenLoco (but not vanilla)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants