godot-extended-libraries / godot-next

Godot Node Extensions - Basic Node Extensions for Godot Engine
MIT License
957 stars 61 forks source link

Cleared Inflector cyclic dependencies #77

Closed Rubonnek closed 3 years ago

Rubonnek commented 4 years ago

All classes have to be global in this case as far as I can tell.

willnationsdev commented 4 years ago

No, this is another case where all of the other classes only exist for the sole purpose of facilitating the Inflector's operations. If someone comes to us with a use case for the other classes and specifically requests that they be separated out to make it easier for them to implement that use case, then I'd consider porting the relevant classes to external scripts. As it stands though, there isn't really a need for it.

Let me know if you believe otherwise, to discuss further.

Rubonnek commented 4 years ago

this is another case where all of the other classes only exist for the sole purpose of facilitating the Inflector's operations

That's absolutely right, but the goal here is not to expose the classes -- that's just a side effect of clearing up the cyclic reference.

I should have explained in detail what's happening here. Let me break this down in sections.

How to reproduce the bug

  1. Open the editor with godot -e --verbose
  2. Attempt to add a node (Ctrl-A), but instead don't add the node and close the dialog.
  3. Close the editor.

You'll see the following output:

ERROR: ~List: Condition "_first != __null" is true.
   At: ./core/self_list.h:112.
ERROR: ~List: Condition "_first != __null" is true.
   At: ./core/self_list.h:112.
WARNING: cleanup: ObjectDB instances leaked at exit (run with --verbose for details).
   At: core/object.cpp:2135.
Leaked instance: GDScriptNativeClass:551
Leaked instance: GDScript:22025 - Resource path: 
Hint: Leaked instances typically happen when nodes are removed from the scene tree (with `remove_child()`) but not freed (with `free()` or `queue_free()`).
Orphan StringName: is_uncountable
Orphan StringName: add_uncountable
Orphan StringName: add_singular
Orphan StringName: Reference
Orphan StringName: _plurals
Orphan StringName: new
Orphan StringName: GDScriptNativeClass
Orphan StringName: get_plurals
Orphan StringName: p_replacement
Orphan StringName: trim_prefix
Orphan StringName: res://addons/godot-next/references/inflector.gd
Orphan StringName: p_rule
Orphan StringName: p_match_ending
Orphan StringName: Rule
Orphan StringName: to_lower
Orphan StringName: _uncountables
Orphan StringName: append
Orphan StringName: add_irregular
Orphan StringName: get_uncountables
Orphan StringName: _singulars
Orphan StringName: build_default_vocabulary
Orphan StringName: has
Orphan StringName: p_singular
Orphan StringName: get_singulars
Orphan StringName: p_word
Orphan StringName: add_plural
Orphan StringName: Vocabulary
Orphan StringName: _init
Orphan StringName: GDScript
Orphan StringName: p_plural

Where's the bug? It's at the beginning and end of the following excerpt -- keep an eye on the Vocabulary keyword:

https://github.com/godot-extended-libraries/godot-next/blob/581f59fce9508616af1caa1eb66d549149fa1cb7/addons/godot-next/references/inflector.gd#L53-L101

The class Vocabulary isn't completely defined at the time it references itself, generating the cyclic reference.

First possible solution

The issue here is the function build_default_vocabulary, which we should be able to fix easily by splitting that code appart and just having a VocabularyFactory like the following:

tool
class_name Inflector
extends Reference
# author: xdgamestudios (adapted from C# .NET Humanizer, licensed under MIT)
# license: MIT
# description: Provides inflection tools to pluralize and singularize strings.
# todo: Add more functionality
# usage:
#   - Creating Inflector:
#       inflector = Inflector.new() # new inflector with empty vocabulary
#       inflector = Inflector.new(vocabulary) # new inflector with vocabulary
#       Note:
#       - Inflector with no vocabulary will not be able to apply convertion out-of-the-box.
#   - Creating Vocabulary:
#       vocabulary = Inflector.Vocabulary.new() # creates new empty vocabulary
#       vocabulary = Inflector.VocabularyFactory.build_default_vocabulary()
#       Note:
#       - Empty vocabulary can be manually configured with custom rules.
#       - Default vocabulary will apply convertions for english language.
#   - Get Vocabulary:
#       vocabulary = inflector.get_vocabulary() # reference to the vocabulary used by the inflector.
#   - Add Rules:
#       vocabulary.add_plural(rule, replacement) # adds convertion rule to plural
#       vocabulary.add_singular(rule, replacement) # adds convertion rule to singular
#       vocabulary.add_irregular(rule, replacement) # adds irregular convertion
#       vocabulary.add_uncountable(word) # add unconvertable word
#       Note:
#       - 'rule' is a String with regex syntax.
#       - 'replacement' is a String with regex systax.
#   - Using Inflector:
#       inflector.pluralize(word, p_force = false) # returns the plural of the word
#       inflector.singularize(word, p_force = false) # returns the singular of the word
#       Note:
#       - If the first parameter's state is unknown, use 'p_force = true' to force an unknown term into the desired state.

class Rule extends Reference:
    var _regex: RegEx
    var _replacement: String

    func _init(p_rule: String, p_replacement: String) -> void:
        _regex = RegEx.new()
        #warning-ignore:return_value_discarded
        _regex.compile(p_rule)
        _replacement = p_replacement

    func apply(p_word: String):
        if not _regex.search(p_word):
            return null
        return _regex.sub(p_word, _replacement)

class Vocabulary extends Reference:
    var _plurals: Array = [] setget, get_plurals
    var _singulars: Array = [] setget, get_singulars
    var _uncountables: Array = [] setget, get_uncountables

    func get_plurals() -> Array:
        return _plurals

    func get_singulars() -> Array:
        return _singulars

    func get_uncountables() -> Array:
        return _uncountables

    func add_plural(p_rule: String, p_replacement: String) -> void:
        _plurals.append(Rule.new(p_rule, p_replacement))

    func add_singular(p_rule: String, p_replacement: String) -> void:
        _singulars.append(Rule.new(p_rule, p_replacement))

    func add_irregular(p_singular: String, p_plural: String, p_match_ending: bool = true) -> void:
        if p_match_ending:
            var sfirst = p_singular[0]
            var pfirst = p_plural[0]
            var strimmed = p_singular.trim_prefix(sfirst)
            var ptrimmed = p_plural.trim_prefix(pfirst)
            add_plural("(" + sfirst + ")" + strimmed + "$", "$1" + ptrimmed)
            add_singular("(" + pfirst + ")" + ptrimmed + "$", "$1" + strimmed)
        else:
            add_plural("^%s$" % p_singular, p_plural)
            add_singular("^%s$" % p_plural, p_singular)

    func add_uncountable(p_word: String) -> void:
        _uncountables.append(p_word.to_lower())

    func is_uncountable(p_word: String) -> bool:
        return _uncountables.has(p_word.to_lower())

var _vocabulary: Vocabulary setget, get_vocabulary

func _init(p_vocabulary = null) -> void:
    if not p_vocabulary:
        p_vocabulary = VocabularyFactory.build_default_vocabulary()
    else:
        _vocabulary = p_vocabulary

func get_vocabulary() -> Vocabulary:
    return _vocabulary

func pluralize(p_word: String, p_force: bool = false) -> String:
    var result = apply_rules(_vocabulary.get_plurals(), p_word)

    if not p_force:
        return result

    var as_singular = apply_rules(_vocabulary.get_singulars(), p_word)
    var as_singular_as_plural = apply_rules(_vocabulary.get_plurals(), as_singular)

    if as_singular and as_singular != p_word and as_singular + "s" != p_word and as_singular_as_plural == p_word and result != p_word:
        return p_word

    return result

func singularize(p_word: String, p_force: bool = false) -> String:
    var result = apply_rules(_vocabulary.get_singulars(), p_word)

    if not p_force:
        return result

    var as_plural = apply_rules(_vocabulary.get_plurals(), p_word)
    var as_plural_as_singular = apply_rules(_vocabulary.get_singulars(), as_plural)

    if as_plural and p_word + "s" != as_plural and as_plural_as_singular == p_word and result != p_word:
        return p_word

    return result

func apply_rules(p_rules: Array, p_word: String):
    if not p_word:
        return null

    if _vocabulary.is_uncountable(p_word):
        return p_word

    var result = p_word
    for i in range(len(p_rules) - 1, -1, -1):
        result = p_rules[i].apply(p_word)
        if result:
            break

    return result
class_name VocabularyFactory
extends Reference

# author: xdgamestudios (adapted from C# .NET Humanizer, licensed under MIT)
# license: MIT

static func build_default_vocabulary() -> Inflector.Vocabulary:
    var vocabulary = Inflector.Vocabulary.new()

    # Plural rules.
    vocabulary._plurals = [
            Inflector.Rule.new("$", "s"),
            Inflector.Rule.new("s$", "s"),
            Inflector.Rule.new("(ax|test)is$", "$1es"),
            Inflector.Rule.new("(octop|vir|alumn|fung|cact|foc|hippopotam|radi|stimul|syllab|nucle)us$", "$1i"),
            Inflector.Rule.new("(alias|bias|iris|status|campus|apparatus|virus|walrus|trellis)$", "$1es"),
            Inflector.Rule.new("(buffal|tomat|volcan|ech|embarg|her|mosquit|potat|torped|vet)o$", "$1oes"),
            Inflector.Rule.new("([dti])um$", "$1a"),
            Inflector.Rule.new("sis$", "ses"),
            Inflector.Rule.new("([lr])f$", "$1ves"),
            Inflector.Rule.new("([^f])fe$", "$1ves"),
            Inflector.Rule.new("(hive)$", "$1s"),
            Inflector.Rule.new("([^aeiouy]|qu)y$", "$1ies"),
            Inflector.Rule.new("(x|ch|ss|sh)$", "$1es"),
            Inflector.Rule.new("(matr|vert|ind|d)ix|ex$", "$1ices"),
            Inflector.Rule.new("([m|l])ouse$", "$1ice"),
            Inflector.Rule.new("^(ox)$", "$1en"),
            Inflector.Rule.new("(quiz)$", "$1zes"),
            Inflector.Rule.new("(buz|blit|walt)z$", "$1zes"),
            Inflector.Rule.new("(hoo|lea|loa|thie)f$", "$1ves"),
            Inflector.Rule.new("(alumn|alg|larv|vertebr)a$", "$1ae"),
            Inflector.Rule.new("(criteri|phenomen)on$", "$1a")
            ]

    # Singular rules.
    vocabulary._singulars = [
            Inflector.Rule.new("s$", ""),
            Inflector.Rule.new("(n)ews$", "$1ews"),
            Inflector.Rule.new("([dti])a$", "$1um"),
            Inflector.Rule.new("(analy|ba|diagno|parenthe|progno|synop|the|ellip|empha|neuro|oa|paraly)ses$", "$1sis"),
            Inflector.Rule.new("([^f])ves$", "$1fe"),
            Inflector.Rule.new("(hive)s$", "$1"),
            Inflector.Rule.new("(tive)s$", "$1"),
            Inflector.Rule.new("([lr]|hoo|lea|loa|thie)ves$", "$1f"),
            Inflector.Rule.new("(^zomb)?([^aeiouy]|qu)ies$", "$2y"),
            Inflector.Rule.new("(s)eries$", "$1eries"),
            Inflector.Rule.new("(m)ovies$", "$1ovie"),
            Inflector.Rule.new("(x|ch|ss|sh)es$", "$1"),
            Inflector.Rule.new("([m|l])ice$", "$1ouse"),
            Inflector.Rule.new("(o)es$", "$1"),
            Inflector.Rule.new("(shoe)s$", "$1"),
            Inflector.Rule.new("(cris|ax|test)es$", "$1is"),
            Inflector.Rule.new("(octop|vir|alumn|fung|cact|foc|hippopotam|radi|stimul|syllab|nucle)i$", "$1us"),
            Inflector.Rule.new("(alias|bias|iris|status|campus|apparatus|virus|walrus|trellis)es$", "$1"),
            Inflector.Rule.new("^(ox)en", "$1"),
            Inflector.Rule.new("(matr|d)ices$", "$1ix"),
            Inflector.Rule.new("(vert|ind)ices$", "$1ex"),
            Inflector.Rule.new("(quiz)zes$", "$1"),
            Inflector.Rule.new("(buz|blit|walt)zes$", "$1z"),
            Inflector.Rule.new("(alumn|alg|larv|vertebr)ae$", "$1a"),
            Inflector.Rule.new("(criteri|phenomen)a$", "$1on"),
            Inflector.Rule.new("([b|r|c]ook|room|smooth)ies$", "$1ie")
            ]

    # Irregular rules.
    vocabulary.add_irregular("person", "people")
    vocabulary.add_irregular("man", "men")
    vocabulary.add_irregular("human", "humans")
    vocabulary.add_irregular("child", "children")
    vocabulary.add_irregular("sex", "sexes")
    vocabulary.add_irregular("move", "moves")
    vocabulary.add_irregular("goose", "geese")
    vocabulary.add_irregular("wave", "waves")
    vocabulary.add_irregular("die", "dice")
    vocabulary.add_irregular("foot", "feet")
    vocabulary.add_irregular("tooth", "teeth")
    vocabulary.add_irregular("curriculum", "curricula")
    vocabulary.add_irregular("database", "databases")
    vocabulary.add_irregular("zombie", "zombies")
    vocabulary.add_irregular("personnel", "personnel")

    vocabulary.add_irregular("is", "are", true)
    vocabulary.add_irregular("that", "those", true)
    vocabulary.add_irregular("this", "these", true)
    vocabulary.add_irregular("bus", "buses", true)
    vocabulary.add_irregular("staff", "staff", true)

    # Uncountables.
    vocabulary._uncountables = [
            "equipment",
            "information",
            "corn",
            "milk",
            "rice",
            "money",
            "species",
            "series",
            "fish",
            "sheep",
            "deer",
            "aircraft",
            "oz",
            "tsp",
            "tbsp",
            "ml",
            "l",
            "water",
            "waters",
            "semen",
            "sperm",
            "bison",
            "grass",
            "hair",
            "mud",
            "elk",
            "luggage",
            "moose",
            "offspring",
            "salmon",
            "shrimp",
            "someone",
            "swine",
            "trout",
            "tuna",
            "corps",
            "scissors",
            "means",
            "mail"
            ]

    return vocabulary

First possible solution consequences

Now that the first possible solution is implemented, attempting to reproduce the issue again (i.e. just pressing Ctrl-A and closing the editor) will yield the following:

ERROR: load: Resource: 'res://addons/godot-next/references/inflector/inflector.gd' is already being loaded. Cyclic reference?
   At: core/io/resource_loader.cpp:360.
SCRIPT ERROR: GDScript::reload: Parse Error: The class "Inflector" was found in global scope, but its script couldn't be loaded.
   At: res://addons/godot-next/references/inflector/vocabulary_factory.gd:7.
ERROR: reload: Method failed. Returning: ERR_PARSE_ERROR
   At: modules/gdscript/gdscript.cpp:599.
SCRIPT ERROR: GDScript::reload: Parse Error: The class "VocabularyFactory" couldn't be fully loaded (script error or cyclic dependency).
   At: res://addons/godot-next/references/inflector/inflector.gd:157.
ERROR: reload: Method failed. Returning: ERR_PARSE_ERROR
   At: modules/gdscript/gdscript.cpp:599.
SCRIPT ERROR: GDScript::reload: Parse Error: The class "Inflector" couldn't be fully loaded (script error or cyclic dependency).
   At: res://addons/godot-next/references/inflector/vocabulary_factory.gd:7.
ERROR: reload: Method failed. Returning: ERR_PARSE_ERROR
   At: modules/gdscript/gdscript.cpp:599.

And what's happening here is that VocabularyFactory has to reference Inflector to access its Vocabulary and Rules subclasses, but Inflector also references VocabularyFactory meaning that the definition of VocabularyFactory is referencing itself quite implicitly.

Final Possible Solution

The only way I see around this issue is to split all these classes apart so that there are no instances of Inflector.Rule nor Inflector.Vocabulary within VocabularyFactory.

I hope this clears up doubts on this pull request.

Rubonnek commented 3 years ago

These cyclic references are are now fixed upstream in the GDScript parser itself.