Code quality could be better

[Jagholin]

I know that the bar for GDScript code is not very high, and I consider myself quite tolerant when it comes to other's people code, but there are a couple of issues here that make me not want to even touch this plugin:

1. You like using Dictionarys way too much. I feel like 99% of the dictionaries in this code should just be replaced with classes for better readability and maintainability
2. You inherit the classes that you have from very strange bases, like PackedDataContainer. I'm not sure you understand what this class even does, but inheriting from it does absolutely nothing in this context. You should inherit from Resource or better yet RefCounted if you don't need any of the Resource's functionality.

There are other strange things that I noticed, but those are relatively minor. Fixing just these 2 should not take a lot of time and will immediately improve code quality by an order of magnitude, from my point of view at least

[Dark-Peace]

When it comes to data structures, the priority is optimisation. I'm not sure if using classes would do any good.

The inheritance only serves one purpose. When you click on the inspector button to add a resource, it shows you the list of compatible resources. If the resources didn't have a common parent, it would show you the entire list of Godot resources that you'd have to scroll through. The issue is that when the plugin was created, Godot didn't support exporting custom resources. In order to do that, the parent had to be a native Godot resource. This is why I chose as parent unrelated resources. I tried to change it in the last version, however if you change the parent class, Godot resets every data associated with it. All previous users would lose everything they made, which is not good.

[Jagholin]

There is no world where using dictionaries (HashMaps) instead of classes gives you any performance benefits. Classes have compact static layout as a rule, and even in the worst case they are just implemented as the same hash maps under the hood. You are sacrificing clarity and type checker protections to get either absolutely nothing(best case) or reduced performance(worst case).

If the changing base class of a resource results in data loss, this is a bug, because it shouldn't happen. Resource system in godot is very flexible in regards to changes, and even if you change the base class, godot doesn't care and doesn't reset your data.

[Jagholin]

And if you want proof that Dictionaries are in fact worse in terms of performance, here you go a little test:

func test_normal_class_init() -> Array:
    var objsArray := []
    var perfBefore := Time.get_ticks_usec()
    for i in 10000:
        var new_obj := TestClass.new()
        new_obj.member_str_1 = "hi" 
        objsArray.push_back(new_obj)
    var timePassed := Time.get_ticks_usec() - perfBefore
    print("test_normal_class_init perf is ", timePassed)
    return objsArray

func test_dict_init() -> Array:
    var objsArray := []
    var perfBefore := Time.get_ticks_usec()
    for i in 10000:
        var new_obj := TestClass2.make_dictionary()
        new_obj["member_str_1"] = "hi" 
        objsArray.push_back(new_obj)
    var timePassed := Time.get_ticks_usec() - perfBefore
    print("test_dict_init perf is ", timePassed)
    return objsArray

func test_class_access(clarr: Array):
    var perfBefore := Time.get_ticks_usec()
    var s := 0
    for i in 10000:
        var t = clarr[i].member_var_10 + clarr[i].member_var_12
        s += t
    var timePassed := Time.get_ticks_usec() - perfBefore
    print ("test_class_access perf is ", timePassed, " res ", s)

func test_dict_access(clarr: Array):
    var perfBefore := Time.get_ticks_usec()
    var s := 0
    for i in 10000:
        var t = clarr[i]["member_var_10"] + clarr[i]["member_var_12"]
        s += t
    var timePassed := Time.get_ticks_usec() - perfBefore
    print ("test_dict_access perf is ", timePassed, " res ", s)

func _ready():
    var clObjs := test_normal_class_init()
    var dictObjs := test_dict_init()
    test_class_access(clObjs)
    test_dict_access(dictObjs)

test_class.gd: https://pastebin.com/Cg99WRuG test_class_2.gd: https://pastebin.com/KaVUkq1S

Results:

test_normal_class_init perf is 86306
test_dict_init perf is 82488
test_class_access perf is 3321 res 0
test_dict_access perf is 6537 res 0

Creating objects has slightly more overhead(not really surprising, due to constructors being run) When it comes to data access, class objects are 2x better in this test than dictionaries

[Dark-Peace]

Interesting. I'll probably make that change one day.