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.

Sign up for GitHub

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

Jump to bottom

Make changes to allow Windows to run under a debug environment #804

Merged
merged 2 commits into from
Nov 3, 2022
Merged

Make changes to allow Windows to run under a debug environment #804

merged 2 commits into from
Nov 3, 2022

Conversation

Jettford
Copy link
Collaborator

@Jettford Jettford commented Nov 1, 2022 •
edited
Loading

  • Replace all interaction of std::string and sqlString.

@Jettford
Make changes to certain database functions and a debug assert
0a8c6e5 
- Replace all interaction of std::string and sqlString.
- Add a return before a debug assertion can be triggered by lvl chunks being loaded on server start.
@Jettford
Merge branch 'main' into windows-debug-fixes
70289d1
EmosewaMC
EmosewaMC approved these changes Nov 2, 2022
View reviewed changes
Copy link
Collaborator

@EmosewaMC EmosewaMC left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me. Are there more of these in the codebase? If there are more we may want to consider changing them as well in the future just to be consistent and not have this issue pop-up in the future.

@aronwk-aaron
Copy link
Member

aronwk-aaron commented Nov 2, 2022 •
edited
Loading

Looks good to me. Are there more of these in the codebase? If there are more we may want to consider changing them as well in the future just to be consistent and not have this issue pop-up in the future.

I did a brief overview and there are still some in some distance corners of the code base that use std::string.
I can take a look at updating them and adding to this PR if wanted

@Jettford
Copy link
Collaborator Author

Jettford commented Nov 2, 2022

There definitely are more but these seem to be the only ones causing issues. This is some definite black magic bugginess.

aronwk-aaron
aronwk-aaron approved these changes Nov 2, 2022
View reviewed changes
Copy link
Member

@aronwk-aaron aronwk-aaron left a comment

Choose a reason for hiding this comment

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

since this is only a debug issue and does not effect release, I'll go ahead an approve it. If other places cause issues, we can just update them later

@aronwk-aaron aronwk-aaron merged commit b974eed into main Nov 3, 2022
@Jettford Jettford deleted the windows-debug-fixes branch December 17, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EmosewaMC EmosewaMC EmosewaMC approved these changes

@aronwk-aaron aronwk-aaron aronwk-aaron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jettford @aronwk-aaron @EmosewaMC