-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Research (aka Technologies) #845
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/research.cpp
Outdated
| if (type->category() == research_category::age_advance) { | ||
| type->owner.advance_age(); | ||
|
|
||
| } else if (type->category() == research_category::age_advance) { |
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.
we can do the research type-specific directly with nyan by testing the patch hierarchy:
// this procedure may of course change, but the principle will most likely remain.
nyan::Object research = ...; // the research nyan object that now shall apply
// apply the research patch
nyan::Transaction tx = database.transaction();
for (auto &patch : research.get<nyan::Patch>("patches")) {
tx.add(research);
}
tx.apply();
if (research.extends("openage.research.AgeAdvance")) {
// do things for age advancements that nyan didn't patch (i don't know what that would be, maybe gui updates?)
}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'm back)
EDIT: This was actually a typo: advance_age is handled at the first if, this was supposed to be generic, thinks that couldn't be handled just with nyan (eg. research that allows to see the enemy line of sight).
However my following explanation for the advance_age still stands
The code you posted will be
inside NyanResearchType::apply ->
which ovverides ResearchType::apply ->
which is called inside Reaseach::apply ->
which is called from the ReaseachAction::update
Notes:
ResearchType immutable (contains the name, cost, patch ...)
Reaseach muttable (is active, is completed ...)
Based on that when the gui will have to draw the progress bar at the top of the screen it need only action.research.get_categroy() == advance_age and action.timer.get_progress()
No need for the gui to dive into the nyan hierarchy...
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.
Welcome back :)
Ah now I get it. Yes, we need to implement some research types in the engine, e.g. the line of sight. This would be done by specifying an object layout in nyan, which is then used by the c++-part to evaluate the data changes, e.g. the line of sight or whatever is needed. The switching of which c++-part to use still has to be done with a nyan-query, though.
The call hierarchy is correct I'd say. It would be a nice abstraction over the nyan backend.
I also like the difference between Research and ResearchType (i.e. ongoing vs its type).
To make the gui nyan-independent should be a high goal :D
This wrapping of nyan-types is a great idea to write the interface to nyan: Then we'd have C++-classes that do the intended access to nyan objects (=> they're tightly coupled) and the rest-code can just use the C++-interface. The way it should be done.
9877c05 to
7a332c9
Compare
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.
Good improvements :)
We should really think about how we reorganize the gamelogic for the event-based engine.
We wrote down some ideas here:
https://pad.stusta.de/p/openage-architecture-v2
For discussing ideas etc, please come join our chatroom :)
|
|
||
| int triggers; | ||
|
|
||
| }; |
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 think this should go into a separate file. All those monster files deserve to be split up, actually :)
This timer triggering should be synched with @Tomatower's event triggers, where such a timer would be registered and you'd get a function call for the right time. Might be a very good start for integrating into the event-architecture.
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.
@Tomatower code is far away and huge changes will have to be made in order to be able to move into curves (I will write more at his pr)
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.
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.
The events are actually designed for that, and the only thing that is missing is a nice Api in the game engine - hopefully without major changes to the existing codebase - so inspire me for the best possbile api 😃
libopenage/unit/attribute.h
Outdated
| Attribute(UnitType *type, coord::phys_t r, coord::phys_t h, typeamount_map d) | ||
| Attribute(UnitType *type, coord::phys_t r, coord::phys_t h, typeamount_map d, | ||
| coord::phys_t min_range = 0, bool friendly_fire = false, | ||
| coord::phys_t area_of_effect = 0) |
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.
indent, and default value assignments don't have spaces around the = please
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 to me.
libopenage/unit/action.cpp
Outdated
| interval{interval}, | ||
| max_triggers{max_triggers}, | ||
| time_left{interval}, | ||
| triggers{triggers} { |
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.
where does triggers come from? currently its the uninitialized member.
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.
Why can this even compile? It must obviously be zero
Classes
Note: Support for repeating research