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

Fix trade crash in certain situations #52042

Merged
merged 2 commits into from
Oct 3, 2021

Conversation

dseguin
Copy link
Member

@dseguin dseguin commented Oct 2, 2021

Summary

None

Purpose of change

Fixes #52014

Describe the solution

Store the item_location itself instead of storing a copy of the pointer.

Describe alternatives you've considered

There's probably some pointer magic that could sort this out as well, but this seems like the safer option.

Testing

  1. Load save file from the issue
  2. Attempt to trade using items from various locations (vehicle cargo, on person, inside container, etc.)
  3. No more seg faults

Additional context

@BrettDong BrettDong added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` NPC / Factions NPCs, AI, Speech, Factions, Ownership labels Oct 2, 2021
@ferociousdork
Copy link
Contributor

If you wish, you can change:

for( item_pricing ip : sorted_stuff ) {

to capture by reference as well. That copy is the reason it crashed in the first place.

Co-authored-by: ferociousdork <[email protected]>
@dseguin
Copy link
Member Author

dseguin commented Oct 3, 2021

Cheers, I would have never thought of that. I've updated the solution.

@kevingranade kevingranade merged commit 1b8564a into CleverRaven:master Oct 3, 2021
@dseguin dseguin deleted the fix_trade_crash branch November 7, 2021 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segmentation fault when trading with Eddie Isherwood from the Isherwood ranches.
4 participants