Line Data

[RiOnE03]

Review Code......

[TheDuriel]

So this is actually a perfect example of what I wanted to avoid:

var s: String = "@onready var %s: %s = %s" % [line_data.get_arg(0,"place_holder"), line_data.get_arg(1,"Node"), line_data.get_arg(2,"Node.new()")]

The purpose of LineData was to hide away the complexity of argument counts, order, and nature. And to account for user error and convenience.

Now if a user provides the wrong order or number of arguments, the macro will either fail or produce an unusable line.

In my very first version of this plugin, Macros themselves were responsible of doing error validation for this. But it produced a lot of redundant and ugly code. Completely defeating the 'make macros easily' goal I had in mind.

So I'm not sure if these changes are a step in the right direction for easy of use and extendibility.

More code for the same output is never acceptable imho. :/

[TheDuriel]

Additionally, this now prevents the implementation of more interesting macros. Like the "vars" macro I mentioned in the earlier review.

This method no longer permits for variable numbers of arguments to be passed for that purpose.

[TheDuriel]

Take for example this:

vars one two three int

var one: int = 0
var two: int = 0
var three: int = 0

vars int one two

var one: int = 0
var two: int = 0

vars int one 1 two

var int: one = 1
var int: two = 1

While the third example is a bit extreme.

The current implementation of LineData would permit for this. (assuming type detection is enhanced)

[TheDuriel]

Or even crazier, but something I'd actually use:

vars one int two float three vec2

[RiOnE03]

This code can't throw errors the first argument in the function asks the element it needs and the second arguments defines the default value it should take if the element is not provided, infact this approach is not complex at all and the second argument can be skipped which will return default as value which your code was also handling by returning placeholders without any input. and as for the complex structures. Indeed it won't handle those until you define specific code structure in the macro. I don't think even your approach can can handle that instinctively without defining specific structure. Also allow me to demonstrate how I can achieve your complex vars with my code. Although its not as robust to handle like yours is doing.

[TheDuriel]

Here is an example of the vars macro.

@tool
extends MagicMacrosMacro

const ALIASES: Array[String] = ["vars", "vs"]

static func is_macro_alias(arg: String) -> bool:
    return arg in ALIASES

static func apply_macro(line_data: MagicMacrosLineData) -> String:
        # This isn't needed. I'd add a helper in LineData that returns a copy of these arrays.
    var identifiers: Array[String] = line_data.identifier_args.duplicate()
    var types: Array[String] = line_data.type_args.duplicate()
    var values: Array[String] = line_data.remainder_args.duplicate()

    var s: String = ""

    for idx: int in identifiers.size():
        var identifier: String = identifiers[idx]
        var type: String = types[min(idx, types.size() - 1)]
        var value: String = values[min(idx, values.size() - 1)]

        var ss: String = "var %s: %s = %s" % [identifier, type, value]
        s += ss
        if not idx == identifiers.size() - 1:
            s += "\n"

    return s

[RiOnE03]

@tool
extends MagicMacrosMacro

const ALIASES: Array[String] = ["vars"]

static func is_macro_alias(arg: String) -> bool:
    return arg in ALIASES

static func apply_macro(line_data: MagicMacrosLineData) -> String:
    var s: String = ""
    if line_data.arguements.size()<4:
        s = "var %s: %s = %s" % [line_data.get_arg(0,"place_holder"), line_data.get_arg(1,"int"), line_data.get_arg(2,"0")]
    else:
        var last_index: int = line_data.arguements.size()-1
        for i in line_data.arguements.size()-2:
            s+= "var %s: %s = %s\n\n" % [line_data.get_arg(i,"val"+str(i)), line_data.get_arg(last_index-1,"int"), line_data.get_arg(last_index,"0")]
    return s

Here is my example Also this is a personal recommendation it is completely not mandatory to merge this. I just wanted to share.

[TheDuriel]

Yeah I don't feel like this is an enhancement to the plugin at this time. Sorry!

I've pushed a fix to type detection in the meantime. As it is only 3 built in types that were being potentially missed.

[RiOnE03]

Here is another example of something that I wanted Code:

@tool
extends MagicMacrosMacro

const ALIASES: Array[String] = ["vars"]

static func is_macro_alias(arg: String) -> bool:
    return arg in ALIASES

static func apply_macro(line_data: MagicMacrosLineData) -> String:
    var s: String = ""
    if line_data.arguements.size()<4:
        s = "var %s: %s = %s" % [line_data.get_arg(0,"place_holder"), line_data.get_arg(1,"int"), line_data.get_arg(2,"0")]
    else:
        var last_index: int = line_data.arguements.size()-1
        var ind: int = line_data.arguements.size()-2
        if line_data.get_arg(1).is_valid_int():
            ind = line_data.get_arg(1).to_int()
            for i in ind:
                s+= "var %s: %s = %s\n\n" % [line_data.get_arg(last_index+1,"val"+str(i)), line_data.get_arg(last_index-1,"int"), line_data.get_arg(last_index,"0")]
        else:
            for i in ind:
                s+= "var %s: %s = %s\n\n" % [line_data.get_arg(i,"val"+str(i)), line_data.get_arg(last_index-1,"int"), line_data.get_arg(last_index,"0")]
    return s

And its completely fine. I only meant it as a recommendation its not important to merge this or anything.

[RiOnE03]

I am thinking of keeping this repo as a copy of yours. and let it be as an alternate.