skript-parser icon indicating copy to clipboard operation
skript-parser copied to clipboard

Add database support

Open TheLimeGlass opened this issue 1 year ago • 2 comments

Added database support! Yay! I was the one to make the new variable system in Skript, so I have extensive experience with how variables are handled and saved. That knowledge helped me make this pull request.

This pull request keeps support for extensibility of projects using skript-parser. Like my project Scroll. Example of what I mean is the usage of including SkriptLogger in important potential user written configurations errors.

Required changes:

  • I choose GSON for the serialization system. Let me know everyone's opinion. Had to gradle shadow library.
  • Removed clearVariables method in Variables because that's a security risk allowing static access to clear the entire variables cache.
  • Option node system could be changed or improved? The current OptionLoader class is locked down to SectionConfiguration, so I can't utilize it as FileSection is unrelated to SectionConfiguration, and no parser reference. So I did split parsing inside FileSection with FileSection#get

TheLimeGlass avatar Mar 27 '24 05:03 TheLimeGlass

I don't know why the tests are failing. I don't believe it's related to this pull request. Tests were successful on my system.

TheLimeGlass avatar Mar 27 '24 05:03 TheLimeGlass

Very nice changes! Thanks for the contribution.

Can you please elaborate more on your problem with FileSection/OptionLoader? I have not had the time to properly review your code, but it seems strange to use these two combined in a situation they were not designed for?

Skript uses it's own built in system for reading the config.sk configuration file. skript-parser does not have a fully fledged configuration system, so i'm make-shifting the FileSection and FileElement system to allow for this usage.

I think it works as it stands, but essentially being able to parse a String list as a FileSection that can get nodes that are FileElements with key-node format. I did that sorta in this pull request, but it's not built for that, which is why I added that statement.

TheLimeGlass avatar Mar 27 '24 19:03 TheLimeGlass