search

OrderOfThePorcupine / ProjectPorcupine

Project Porcupine: A Base-Building Game...in Space!
GNU General Public License v3.0
299 stars 89 forks source link

Validation of all inputs #137

Open kd7uiy opened 5 years ago

kd7uiy commented 5 years ago

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.

NogginBops commented 5 years ago

Do we really need scripts? We have C# modding support, so there really is no need for scripting. Lua does not have type checking plus it has a lot of runtime overhead. Wouldn't it just be simpler to scrap Lua?

kd7uiy commented 5 years ago

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.

NogginBops commented 5 years ago

I think I might have misunderstood the issue. Are we talking about validating the JSON data? It's not very clear from the issue

kd7uiy commented 5 years ago

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.

kd7uiy commented 5 years ago

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

NogginBops commented 5 years ago

This is another point in favor of scrapping Lua, you can't do good static analysis and verification that the code will even run

kd7uiy commented 5 years ago

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.