-
-
Notifications
You must be signed in to change notification settings - Fork 183
[nfoSceneParser] new config options #586
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
[nfoSceneParser] new config options #586
Conversation
Could you please bump the version in the .yml file? |
From an initial review it looks good. I will test and review more thoroughly when I get home from work today. |
I will greatly appreciate that. Although I tried to not change the default behavior, the last thing I would like to do is to break something for existing users. Update process will overwrite existing |
If they update fully, yes they will need to redo their config. If it was me I would create a backup and copy over the settings or just copy over your changes to my local version but it's going to be up to the user. TBH I feel like this isn't used a lot so most people won't bother updating lol |
Ok so just in case I added some protection to make sure the new version works with old config file that doesn't have these new properties. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting changes as technically my request is not a requirement, just a small improvement but it can be merged as is and shouldn't have any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Added possibility for nfoSceneParser plugin to configure:
Additionally added: