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 unity settings file #2303

Closed
wants to merge 3 commits into from
Closed

Fix unity settings file #2303

wants to merge 3 commits into from

Conversation

wolf843
Copy link

@wolf843 wolf843 commented Nov 8, 2019

test pull

@msftclas
Copy link

msftclas commented Nov 8, 2019

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ wolf843 sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@rajat2004
Copy link
Contributor

rajat2004 commented Dec 2, 2019

Hey @wolf843, does this also fix the problem of an extra settings.json file getting created in the main directory if the settings.json in Documents/AirSim is not present?
The diff is pretty big so it's a bit difficult to follow

I have opened another PR with some other improvements to Unity build (#2328), and noticed this problem during that

}

// Verify SettingsVersion is greater than 1.2
if (SettingsVersion != 1.2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be < here

@rajat2004
Copy link
Contributor

Hey @wolf843, does this also fix the problem of an extra settings.json file getting created in the main directory if the settings.json in Documents/AirSim is not present?

I think the problem still persists, since the GetAirSimSettingsFileName() is the same as in the older InitializeAirsim.cs

Could be mistaken, please correct me if that's the case

@rajat2004
Copy link
Contributor

@madratman I think the best way to get the settings fixed would be to do piecewise PRs or atleast smaller commits, will make it much easier to review and test

@zimmy87
Copy link
Contributor

zimmy87 commented Mar 12, 2021

closed due to license agreement not being signed and unanswered old CR feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants