0 votes

So, I've been working on a project on-and-off for a few months, and I was just wondering if there was an excessively friendly fellow programmer who could give me a fair critique on a bit of code I've written.

For context, this is code for the Attack State of a player character. Here's a bit of the demo of some of this code in action (In very poor quality :P) :
https://i.imgur.com/2k7GWZs.mp4
*animations are unfinished

I'm coming back to this project today, after a couple months of not being able to really work on it. I've found I've been able to navigate the logic fairly quickly, but I'm worried that the sheer amount of work this one file does is going to become non-navigable as I implement more attacks and special things that attacks can do, like the vaulting I implemented. Is this an inevitability in this sort of project, or is this something I can improve and make more readable and clear with some better organization?

Really appreciate any input on this.

Here's the code:

extends "./Free_Motion_State.gd"
signal vault;

func enter():
    host.state = 'attack';
    track_input = true;
    saveInput(Input);
    pass;

### Prepares next move if user input is detected ###
func saveInput(event):
    if(event.is_action_just_pressed("basic_attack") || event.is_action_just_pressed("special_attack")):
        if(event.is_action_just_pressed("basic_attack")):
            saved_attack = 'basic';
            cur_cost = basic_cost;
        elif(event.is_action_just_pressed("special_attack")):
            saved_attack = 'special';
            cur_cost = spec_cost;
            attack_idx = "";
            if(host.magic_bool):
                magic = "_magic";
            else:
                magic = "";
        if(saved_attack != 'nil'):
            attack_is_saved = true;
    pass;

### Handles all player input to decide what attack to trigger ###
func handleInput(event):
    if(attack_mid || attack_end):
        saveInput(event);
    if(track_input):
        if(host.resource < -10):
            exit_g_or_a();
            return;
        if(event.is_action_pressed("switchL") && event.is_action_pressed("switchR")):
            exit('block');
            return;
        elif(event.is_action_pressed("switchL")):
            type = "precision";
        elif(event.is_action_pressed("switchR")):
            type = "bashing";
        else:
            type = "slashing";

        if(atk_left(event)):
            dir = "horizontal"
            update_look_direction(-1);
        elif(atk_right(event)):
            dir = "horizontal";
            update_look_direction(1);
        elif(host.mouse_enabled):
            dir = "";

        if(atk_down(event) || atk_up(event)):
            if(!atk_left(event) && !atk_right(event)):
                dir = "";
            if(atk_up(event)):
                vdir = "_up";
            elif(atk_down(event)):
                vdir = "_down";
        else:
            vdir = "";
            dir = "horizontal"


        if(host.on_floor()):
            place = "_ground";
        else:
            place = "_air";
        #if an attack is triggered, commit to it
        if(event.is_action_just_pressed("basic_attack") || event.is_action_just_pressed("special_attack")):
            if(event.is_action_just_pressed("basic_attack")):
                current_attack = 'basic';
                cur_cost = basic_cost;
            elif(event.is_action_just_pressed("special_attack")):
                current_attack = 'special';
                cur_cost = spec_cost;
                attack_idx = "";
                if(host.magic_bool):
                    magic = "_magic";
                else:
                    magic = "";

            if(init_attack()):
                return;
        elif(attack_is_saved):
            current_attack = saved_attack;
            if(init_attack()):
                return;
        #cancel the combo
        elif(!attack_is_saved && 
            (!event.is_action_pressed("switchL") && 
            !event.is_action_pressed("switchR") && 
            !event.is_action_pressed("lock")) ):
            if(event.is_action_pressed("left") ||
                event.is_action_pressed("right")):
                exit_g_or_a();
                return;
        elif(!attack_is_saved && !event.is_action_pressed("lock")):
            if(event.is_action_just_pressed("jump")):
                exit_g_or_a();
                return;
    #combo timeout
    if(!attack_start && !attack_mid && combo_end && 
        (!event.is_action_pressed("switchL") && 
        !event.is_action_pressed("switchR") && 
        !event.is_action_pressed("lock"))):
        exit_g_or_a();
        return;
    pass;

### Runs every frame ###
func execute(delta):
    if(!input_testing):
        attack();
    #prevent player slipping
    if(host.on_floor() && !attack_mid && !dashing):
        host.hspd = 0;
    pass;

### Cleans state up when player changes state ###
func exit(state):
    #reset
    host.reset_hitbox();
    stopTimers();
    reset_strings();
    combo_step = 0;
    attack_start = false;
    attack_mid = false;
    attack_end = false;

    dashing = false;
    hit = false;
    attack_spawned = false;
    attack_is_saved = false;
    .exit(state);
    pass;

### Triggers appropriate attack based on the strings constructed by player input ###
func attack():
    #if current_attack has a value, the attack hasn't actually triggered yet, and we're calling the attack to be triggered...
    if(current_attack != 'nil' && !attack_spawned && attack_start):
        #TODO: put this signal in the attacks instead
        host.emit_signal("consume_resource", cur_cost);
        animate = true;
        attack_spawned = true;

        construct_attack_string();

        var path = "res://Objects/Actors/Player/Rose/AttackObjects/" + type + "/" + current_attack + "/";
        #Ignore certain string combinations that result in existing attacks
        if(current_attack == "basic"):
            if(attack_idx == "_1"):
                attack_idx = "_2";
            else:
                attack_idx = "_1";
            magic = "";
            if(dir == "horizontal"):
                if(!atk_up(Input) && !atk_down(Input)):
                    vdir = "";
                    place = "";
                elif(!atk_down(Input)):
                    place = "";
                elif(atk_down(Input) && place == "ground"):
                    attack_idx = "";
            elif(atk_up(Input)):
                place = "";

            path += dir+vdir+place+"_attack.tscn";


        if(current_attack == "special"):
            if(type == "slashing" || type == "bashing"):
                if(dir == "horizontal"):
                    if(!atk_up(Input) && !atk_down(Input)):
                        vdir = "";
                        place = "";
                    elif(!atk_down(Input) || type == "bashing"):
                        place = "";
                elif(atk_up(Input)):
                    place = "";


            path += dir+vdir+place+magic+"_attack.tscn";

        var true_length = cast_length;
        if((atk_down(Input) || atk_up(Input)) && (atk_right(Input) || atk_left(Input))):
            true_length = true_length / sqrt(2);
        if(atk_down(Input)):
            host.get_node("vault_cast").cast_to.y = true_length;
        elif(atk_up(Input)):
            host.get_node("vault_cast").cast_to.y = -true_length;
        else:
            host.get_node("vault_cast").cast_to.y = 0;
        if(atk_right(Input)):
            host.get_node("vault_cast").cast_to.x = true_length;
        elif(atk_left(Input)):
            host.get_node("vault_cast").cast_to.x = -true_length;
        else:
            host.get_node("vault_cast").cast_to.x = 0;

        var effect = load(path).instance();
        effect.host = host;
        effect.attack_state = self;
        host.add_child(effect);
    pass;

### Initializes attack once the player has committed ###
func init_attack():
    host.reset_hitbox();
    if(input_testing):
        construct_attack_string();
        attack_is_saved = false;
        return true;
    else:
        if(current_attack != 'nil'):
            stopTimers();
            attack_start = true;
            combo_step += 1;
            attack_end = false;
            attack_is_saved = false;
            saved_attack = 'nil'
            return true;
        return false;
    pass;

### Constructs the string used to look up attack hitboxes and animations ###
func construct_attack_string():
    var tdir = dir;
    var tvdir = vdir;
    var tcurrent_attack = "";
    if(dir != ""):
        tdir = "_" + dir;
    tcurrent_attack = "_" + current_attack;

    attack_str = type+tdir+tvdir+place+magic+tcurrent_attack;
    print(attack_str);
    pass;
asked Jun 4, 2019 in Projects by tyo (42 points)

To sum up how it works, basically, it uses the player input to construct a string. That string is used to look up both the appropriate hitbox to an attack as well as the animation for the attack. The stuff that isn't doing that is for other functionality, like how long the player can wait before the combo is registered as "finished" and aforementioned vaulting using some particular attacks.

Also, I had to cut non-essential code out to stay within the character limit. If anyone would like to access more of the code, just let me know and I can link the repository I'm storing this in.

Let me suggest a couple of coding tips (if you haven't heard his already, or if you don't already follow these practices). Whenever my code becomes large and unwieldy, I try to move it into other files, i.e. I factor the code. In Godot, it may not be that difficult, due to its node paradigm.

Where I can, I write comments. At times, I'll comment the hell out of code, putting in little examples and diagrams. This really helps when you come back to the code months later.

Please log in or register to answer this question.

Welcome to Godot Engine Q&A, where you can ask questions and receive answers from other members of the community.

Please make sure to read How to use this Q&A? before posting your first questions.