-
Notifications
You must be signed in to change notification settings - Fork 88
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
Validation of all inputs #137
Comments
Do we really need scripts? |
That is a good question, but really it just converts the validation of LUA to the validation of C#. And we need to validate JSON in any case. |
I think I might have misunderstood the issue. Are we talking about validating the JSON data? It's not very clear from the issue |
I'm talking about any text files that are inputed. JSON, XML (Shouldn't still be any, but...), Lua, C#, and it wouldn't even hurt to validate images and sounds. Anything that is in the Streaming Assets folder should be validated somehow, in my opinion, at least to verify it can be loaded. |
I have added checks for the LUA files (Well, all mod files), although I think we still need to do a better job. At the very least, syntax errors are now being flagged and fail the unit test |
This is another point in favor of scrapping Lua, you can't do good static analysis and verification that the code will even run |
What I think I would like to see is add a "Validate" to IPrototypable, which is called immediately after ReadJsonPrototype. It returns a bool, true if valid, false if not valid. We can then go from there. Initially we might just simply return "true" for everything, but as there are problems, such as the one identified in #126, this gives us a way to easily debug this, and prevent it from occurring again. Once that is done, I believe this is good enough, for now, but it is a philosophy we should continue to improve with time. |
We need to add in scripts to test the validity of all inputs. The initial versions might only do essentially syntax testing, but future versions might start to do more complex things. The idea is if someone has done something invalid with a script as a modder, they would like to know ASAP, and in many instances we might have enough information to know it is a problem when we launch.
In addition, as a unit test we should validate all of the scripts that we are uploading as a part of Streaming Assets. The script validity checkers should also check a few specified invalid items to make sure they are working. No longer should we have invalid scripts checked in once this is done.
This includes scripts and data files (Currently JSON). They should be checked when loaded, and as a part of the unit tests for the project.
The text was updated successfully, but these errors were encountered: