Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attributes system improvements #683

Merged
merged 17 commits into from
Feb 6, 2017
Merged

Attributes system improvements #683

merged 17 commits into from
Feb 6, 2017

Conversation

MaanooAk
Copy link
Contributor

@MaanooAk MaanooAk commented Dec 29, 2016

Attributes system improvements goals:

  • Abstract shared and ushared attributes
    • share method
    • copy method (???)
  • Convert attr_map_t into a proper class (Attributes?)
    • implement
      • add attributes.cpp (?)
    • refactor
  • Split attributes and refactor changes
    • hitpoints -> hitpoints + current hitpoints (issue Split the hitpoints attribute #651)
    • building -> building + dropsite (semi-implemented)
    • attack: move attack stance elsewher
  • Change the worker atrribute
    • rename
    • split into unshared and shared
      • carrying resources
      • move worker action graphics to somewhere else ($1)
    • gather rate per resource
    • move attack type (used only for villagers) from attack ($2)
  • Multitype attribute (for unit with subtypes: villages, trebuchets)
    • implement
    • refactor producers and fix $1 and $2
  • Change type bugs
    • use reinitialise instead of reinitialise in order to keep the unshared attributes (eg. life)
  • Actions attribute calls optimize
    • move has/get attribute in constructor (call once and store) (?)
    • move basic operations inside attributes
  • Add doc

Notes

  • All files including attributes.h: unit_type.h, unit.h, action.h
  • Attributes methods:
    • bool add/has/removeAttribute (hide all map methods)
    • Attribute get<attr_type>

Bugs found (and solved) on the way

  • Health reset every time villager changes resources (eg. from wood to stone) (wait... no)
  • One gather rate for all resources
  • Search for new building to build includes enemy's building
  • Towers auto attack
  • ...

@MaanooAk
Copy link
Contributor Author

After a lot of time, I cant find a way to improve the the copypaste copy() methods.

@TheJJ TheJJ added lang: c++ Done in C++ code improvement Enhancement of an existing component labels Dec 29, 2016
@MaanooAk
Copy link
Contributor Author

MaanooAk commented Jan 1, 2017

Issues:

  • Only attribute left to split is attack. It has the attack stance inside which has to go somewhere else. Any suggestions on grouping attack stance with something else in order to avoid a whole attribute only for this?
  • Many of the actions' update methods have many calls to has_attribute and get_attribute. It seems like they need to be reduced... Should we:
    • create pointers to the used attributes in the constructor?
    • move some basic logic from the actions to the attributes? (For future discussion)
    • nothing for now?
  • General feedback?

@MaanooAk
Copy link
Contributor Author

MaanooAk commented Jan 10, 2017

Should I move attack stance inside a formation attribute? This attribute will also store patrol and follow target command's information.
EDIT: done

@MaanooAk MaanooAk changed the title [WIP] Attributes system improvements Attributes system improvements Jan 10, 2017
@MaanooAk
Copy link
Contributor Author

Any comments?

@VelorumS
Copy link
Contributor

You can also take a look at #642. Some of it is about attributes and position. I'm trying to record, serialize, transfer and replay them on the client. That PR has the quickest possible hacks to be able to do that.

Maybe you've got an idea how implementing record/replay has become simpler with your changes (or how to make it simpler, or how to approach the record/replay correctly).

@TheJJ
Copy link
Member

TheJJ commented Jan 23, 2017

Sorry it takes so long, I'll have a look tomorrow hopefully.

@TheJJ
Copy link
Member

TheJJ commented Jan 30, 2017

On a sidenote (as university and exams keep me busy..): The shared attributes are the ones provided by nyan, the per-unit attributes are engine features and stored like we do currently.

@MaanooAk
Copy link
Contributor Author

Still the nyan should define the existence of a unshared attribute (eg. formation, resource, damaged) or somehow connect shared with unshared (eg. speed->formation, worker->resource, hitpoints->damaged).

EDIT: I thought I more weak connection between nyan and attributes was intended (eg. life: 10 -> hitpoints, damaged, armor) (am I correct?)

@TheJJ
Copy link
Member

TheJJ commented Jan 30, 2017

My idea for that was to create nyan files that ship with the openage engine so that the c++ code can assume that members of those objects (from the shipped .nyan files) exist.
For example:

# this file is shipped with the engine
Ability():
    pass

MoveAbility(Ability):
    speed : float;

GatherAbility(Ability):
    max_weight : float;  # maxium weight of resources to carry

UnitType():
    hp : int    # every unit has hp
    abilities : set(Ability)  # additionally, it can do many other things

Game():
    create_units : set((int, UnitType)) = {}

This means that each nyan-UnitType definitely has a hp member.
The "instanciation" is then done by runtime data:

# this file is then loaded at runtime

VillagerMovement(MoveAbility):
    speed = 1.0

VillagerGathering(GatherAbility):
    max_weight = 20.0

Villager(UnitType):
    hp = 40
    abilities |= {VillagerMovement, VillagerGathering}


# registration patche
Register<Game>():
    create_units |= {(20, Villager)}

Now, let's assume we implemented the following in openage:

// our attribute system and unit handling that is done per-actual-unit
// abilities with their attributes are used for per-unit storage of abilities that this unit posesses.

enum class ability_t {
    gather
};

class Ability {
    virtual ability_t get_type() const = 0;
    virtual void tick(Unit *) = 0;

    nyan::Object *type;
};

class GatherAbility : public Ability {
    // we can very likely optimize this function with template fuckery...
    ability_t get_type() const override { return ability_t::gather; }

    void tick(Unit *unit) override {
        // check if we have to go home
        int max_weight = this->type.get<int>("max_weight");
        if (max_weight > this->current_carry_weight) {
            // you get the point...
            unit->go_to_nearest_dropsite();
            return;
        }

        // here, deduct the resources from the gathering spot.
        // etcetc
        // this->current_target.deduct(blablabla)
        // this->current_carry_weight += roflroflrofl;
    }

    float current_carry_weight = 0;
}

std::unique_ptr<Ability> create_ability(nyan::Object *ability_type) {
    if (ability_type->is_child_of("GatherAbility")) {
        return std::make_unique<GatherAbility>(ability_type);
    }

    return nullptr;
}


/**
 * This is the C++ object tied to the nyan object "UnitType".
 * "UnitType" stores shared attributes (e.g. "maximum hp")
 */
class Unit {
    Unit(nyan::Object *type)
        :
        type{type} {}

    template <typename T> Ability *get_ability() {
        return this->abilities[T].get();
    }

    void add_ability(std::unique_ptr<Ability> ability) {
        this->abilities.insert({ability.get_type(), std::move(ability)});
    }

    /** this links to the nyan object that stores the unit type */
    nyan::Object *type;

    /**
     * each unit stores its damage and it dies,
     * when damage > max_hp
     */
    int damage;

    /** each unit always has a position in the world */
    coord::phys3 position;

    /**
     * Additional per-unit abilities and their storage that are added at runtime by nyan.
     */
    std::unordered_map<attr_t, std::unique_ptr<Ability>> abilities;
};


// ... add the unit to the simulation and nyan gives it the damage attribute ...
void init() {
    nyan::Database db{};
    db.load_files(engine_nyanfiles);
    db.load_files(user_mods);

    // this is normally invoked in a more intelligent way, like for each mod
    // that was loaded.
    db.apply_patch("Register");

    Simulation sim;

    // now, the "InitialUnits.units" contains the villager (it was added by "Register")
    // this is used for "game initialization"
    for (auto &creation_tuple : db.query("Game").get<set, nyan::tuple<int, nyan::Object*>>("create_units")) {
        int amount = creation_tuple.get<0>();
        nyan::Object *type = creation_tuple.get<1>();

        Unit unit{type};
        for (auto &ability_type : type.get<set, nyan::Object*>()) {
            // the ability may require per-unit storage, which is created here.
            std::unique_ptr<Ability> ability = create_ability(ability_type);
            if (attribute.get()) {
                unit.add_ability(std::move(ability));
            }
        }
        sim.add(std::move(unit));
    }

    // loop forever, with the callback below
    sim.loop_forever(loop);
}


void loop(Simulation *sim) {
    for (auto &unit : sim->get_all_units()) {
        nyan::Object *unit_type = unit.type;

        // test if the type is a nyan-child of unit. which it very likely is :)
        if (unit_type->is_child_of("Unit")) {
            int villager_hp = unit_type->get<int>("hp");
            log::log(info << "villager has max hp: " << villager_hp);

            if (unit->damage > villager_hp) {
                unit->die();
            }

            for (auto &ability : unit->abilities) {
                ability->tick(unit);
            }
        } else {
            // wtf internal error
        }
    }
}

@TheJJ
Copy link
Member

TheJJ commented Jan 30, 2017

Sorry, the example not got a bit more advanced than it was initially, but this is probably the first time that the dynamic interaction between nyan and c++ was ever documented. Hope it helps :)

@MaanooAk
Copy link
Contributor Author

MaanooAk commented Jan 30, 2017

So the next steps would be:

  • Fix everything
    • Attributes (this pr)
    • Actions
    • Abilities
  • Create the "shipped" nyan files (after everything is working properly)
  • Convert the converted data to nyan
  • Redefine/Recreate the nyan in more modular way (= stop converting)

(I think we should have this kind of task list all over the place)

@TheJJ
Copy link
Member

TheJJ commented Jan 30, 2017

We have to do the transition slowly, so your shared attributes are mainly the nyan placeholder. But to simplify the transition, it would be nice if we could start the separation, if needed with placeholders, right now.

We can't do the switch instantly because it is too complicated and we slowly have to design the nyan tree structure and engine-shipped definitions.

I'm doing my master's thesis about nyan this summer, so it will be done.
We have to check if my above example is indeed a feasible way to do.
Then start to organize the simulation code so that the shared attributes are clearly separated and then replaced, some references to "here be nyan" are already in the code (e.g. libopenage/unit/unit_type.{h,cpp}, which is similar to what i did above, but with one indirection layer more).

Copy link
Member

@TheJJ TheJJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, I think we should go forward and merge this as it is an improvement to the attribute system which does not seem to complicate things for the future, instead even simplifies them (with the shared and unshared containers).

pls move the attributes class to separate files, then it's good i think. sorry for the delay...

* Contains a group of attributes.
* Can contain only one attribute of each type.
*/
class Attributes {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would put the attributes class in a separate file attributes.cpp,h.

@MaanooAk
Copy link
Contributor Author

MaanooAk commented Feb 6, 2017

Moved to separate file and rebased.

@TheJJ TheJJ merged commit 7e618e5 into SFTtech:master Feb 6, 2017
@TheJJ
Copy link
Member

TheJJ commented Feb 6, 2017

👍

@MaanooAk MaanooAk deleted the attrs branch February 6, 2017 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Enhancement of an existing component lang: c++ Done in C++ code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants