-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Repair action & resource bundle #672
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
Conversation
libopenage/unit/action.cpp
Outdated
| this->entity->has_attribute(attr_type::owner) && | ||
| this->entity->has_attribute(attr_type::attack) && | ||
| this->entity->get_attribute<attr_type::attack>().stance != attack_stance::do_nothing) { | ||
| this->entity->has_attribute(attr_type::owner) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed these in the next commit
|
ResourceBoundle could be like this (gist). Check the main for how its used. EDIT: Closed it by accident... |
|
Implemented a ResourceBundle class and used it in player (kept old one with raw type and amount) and unit repair. Next step is to add somewhere (unit type, unit attributes, ?) the cost of the unit. I have zero confidence implementing that so this is as far I can go (= removing WIP). |
|
Sorry for the delay, university has kept me very busy (http://www.move2space.de/MOVE-II/)... However, I just tested your code and I couldn't get my villagers to repair their town center or mill. The resource bundle is a good idea, as it's capable of adding resource types at runtime easily. We'll need that in the future. The cost of the unit is drawn from the gamedata of course, which is accessible through the |
|
I added the repair ability, that should do the work. I added and some other missing abilities while I was there. |
TheJJ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from minor stuff.
As we have quite a few ...Ability and ...Action classes, i think it might be time to pack those together in a separate namespace, so we have names like ability::Heal and action::Repair. This should reduce the verbosity a bit.
libopenage/unit/ability.cpp
Outdated
| } | ||
|
|
||
| void ConvertAbility::invoke(Unit &to_modify, const Command &cmd, bool play_sound) { | ||
| to_modify.log(MSG(dbg) << "invoke repair action"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copypasta :)
|
|
||
| /* | ||
| * initiates a repair action when given a valid target | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls write /** so it's parsed by doxygen
|
|
||
| /* | ||
| * initiates a heal action when given a valid target | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
| * initiates a research | ||
| * TODO implement | ||
| */ | ||
| class ResearchAbility: public UnitAbility { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| * initiates a patrol action when given a valid target | ||
| * TODO implement | ||
| */ | ||
| class PatrolAbility: public UnitAbility { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| /* | ||
| * initiates a convert action when given a valid target | ||
| */ | ||
| class ConvertAbility: public UnitAbility { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
libopenage/unit/action.h
Outdated
| float time; | ||
| float time_left; | ||
|
|
||
| ResourceBundle cost; // calculate and store the cost in the constructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can write /** stores the cost of the repair */ or whatever is more suitable above it so it's parsed by doxygen, too.
|
The separate namespace should be done with a refactoring tool. |
|
You mean templating the attributes? Yes, could also be done :) Shall I merge your stuff before that or do you wanna integrate it also in this pull? |
|
You should merge this (do you want me to rebase?) |
|
No I don't think you need to rebase, it doesn't conflict with the |
|
I just realized I'm creating a ton of conflicts in my new pr (#683) with this one... |
TheJJ
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still can't get it to work, and I think it is because you don't add the repair ability to the villagers. Or does it work for you some other way?
libopenage/unit/ability.cpp
Outdated
| } | ||
|
|
||
| void HealAbility::invoke(Unit &to_modify, const Command &cmd, bool play_sound) { | ||
| to_modify.log(MSG(dbg) << "invoke repair action"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still the wrong message here
|
I see... Just leave it, I will investigate and come back to you. |
|
I'd like to add to this that there's a bug in the original game which still exists where you can repair (and garrison) a building from a diagonal location. It's called the wall corner glitch and I'm wondering if this bug also exists within openage and if it should either be fixed, or stay exactly the same. |
|
@Yanikore I think we'll have the same problem right now because we don't have any blocking physics currently, the pathfinder is really stupid and doesn't account for that. Go and help out at #366 :) |
|
@TheJJ I'd love to help out but I'm not capable for the C++/python side of this. I'm more a PHP developer than anything so I'll probably help out in the future when a website gets made. Right now I can only add some conceptual ideas to the situation. |
|
yay it works :) |
Repair of units based on the doc/reverse_engineering/repair.md (file in pull request).
In order to continue:
A data structure for multiple resource costs should be implemented (game_resource_bundle ?)Can I implement these? or just leave as it is and revisit after the nyan revolution?EDIT: Also game_resource_bundle should replace the
game_resource type, int valuecouples all over the place.