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

Add Validation for Data Json Files with feedback to the end user. #218

Merged
merged 20 commits into from
Jun 24, 2024

Conversation

Kanzaji
Copy link
Contributor

@Kanzaji Kanzaji commented Feb 12, 2024

Hey there 😄
As talked on Discord, here is the PR with data validation for json files and feeback to the end user on what is missing / wrong in the data files. The PR as of writing this is still WIP (Missing Deposit validation), but I decided to put it up for now anyways for easier checks on the progress :D

UPDATE:
This also should resolve: #202 #211

Features

Validation of Data Json Files will stop EE from crashing the game by loading incorrect json files, or creating duplicates of ids etc.

Validation step will print to the log any errors and warnings found in the validation process, with file name present in each message, and additional message if file was marked as not acceptable to the registration.

obraz

How does this even work?

Validation of jsons is done on the loading stage of the jsons, before they are passed to the MC Codec implementation. If validation fails, the json is not decoded to Java object and is not registred in EE registry.

Validators are stored in a map of String -> BiFunction in each json model class. If some model is a "sub-model" of another model, aka: Material has field with model of MaterialProperties, you can pass that map to the validator, and it will validate all fields of that object separetely.

All fields are stored in the Validator Map with key being a field name, and value a BiFunction used as a validator.


If a field that doesn't exist in that map is found, a warning about unknown field will be printed.
obraz


If a required field is missing, a validation of the json will fail, and missing fields are going to be printed to the log.
obraz

Validator class

Validator class is a "utility" class used to get basic validation BiFunctions, customized with name passed in the class constructor.
Name of the Validator has to be exactly the same as expected field to validate with use of newly created Validator!

It contains from simple "Field is required" to Validation of MC ResourceLocation syntax, Number Range and validation of other objects.

Accessing data from other fields

Note: This was something I originally didn't plan on, and that was stupid decision :P So this system isn't perfect, but it does the job 😅

Accesssing data from the main object or the parent is done adding a suffix _rg -> Request Original, to the id in the Validators map of the model. BiFunction of Validator to validate obects will pass to the validator entire object instead of the field name of this validator.


Accessing parent information (So, Material.Properties.isBurnable from Material.Colors.gasColor for example) requires passing the original object from the beginning to all validators of the Colors Model. Usage of _rg is required, but prohibited is the usage of Validator#validateObject(JsonElement, Path, Map<...>), due to errors caused by missing fields in the parent object.

For passing parent object, required is use of Validator#getPassParentToValidators(Map<String, BiFunction<JsonElement, Path, Boolean>>, boolean) which passes the original object to the validators in additonal "TEMP" field.
This validator does not support arrays of arrays on any primite values. Can be used only when validated is meant to be Array of Objects or simply, Object.

It will launch validation as normal (aka validateObject without _rg suffix), and access to TEMP field can be gained with use of _rg suffix in the validators of the model that requires it.
Example can be seen in Material model.

ToDo:

  • Universal Validation class (Validator)

Validation of Strata.

  • Validation of simple stuff
  • Check if stata with X ID already exists.

  • Validation of Compat.

Validation of Material and it's sub-modules.

  • Validation of main model
  • Validation of sub-models.
  • Check if Material with X ID already exists.

Validation of Deposists and it's sub-modules.

  • Validation of the type
  • Validation of each sub-type
  • Validation of the objects

Code cleanup 😅

  • Remove checks for log, Logger was replaced with simple wrapper that removes any warning or error calls if logConfigErrors is set to fales in the config file.
  • Some general code cleanup

@Kanzaji Kanzaji marked this pull request as ready for review February 18, 2024 10:34
@Kanzaji
Copy link
Contributor Author

Kanzaji commented Feb 18, 2024

Alrighty, except few of the question I wrote to you Rid on discord, this PR is ready for the review and testing on your side ❤️

One last task that I will do before merging:

  • Move DepositValidators#getMaxYLevelValidation() to Validator Class to be used as a standalone validator. Cleanup required code due to this change.
  • Change few things depending on the response to my questions 😅

@Kanzaji
Copy link
Contributor Author

Kanzaji commented Mar 28, 2024

Okay I would say this is ready for merge and testing on your side ❤️

Just last question, as currently colors, if present, are going to mention "x should be set when x is present in processed types", for example, for gasColor.
I would say this is neat idea, but I don't think I have a way to know / guess if the material is actually tint-based. Should this stay or it should verify if the color value is correct and don't mention anything 😅

@Ridanisaurus Ridanisaurus merged commit 0db4f33 into Ridanisaurus:EEV2-1.19-Dev Jun 24, 2024
@Kanzaji
Copy link
Contributor Author

Kanzaji commented Oct 8, 2024

PS: For anyone that sees this PR - This entire system was reworked on 1.21 branch, this is a good idea but this execution of it isn't the best 😄

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

Successfully merging this pull request may close these issues.

2 participants