Hi, I slowly started moving towards making adv map objects more configurable. Note that for proper “configurable object” we need still scripting and map editor. Neither of those are in my plans - I’m mostly interested in removing hardcoded configurations for extremely similar objects - e.g. moving all ID-based checks from such objects like CGPickable. So instead of checking for object type all such data can be read from configuration. Ideally I’d like to apply this idea to ~20 objects such as School of War, Windmill, Temple, Warrior Tomb, maybe even Seer Huts, Events and Keymasters.
For now my plan looks like this:
Move all id-dependent code into initialization and add fields like “messageID” or “soundID” to objects. This will help me to see all object-specific constants. (that should be moved to config in the end)
Generalize “things” granted to player in such objects. So instead of having object-specific code that grants something to player (resources, skills, artifacts) there will be generic “give reward” method that can use any data from object configuration. In the end it should be possible to use such method for complex objects like events/seer hut rewards.
Generalize what makes object visited. Right now we have multiple sets of objects with only difference between them is who can visit objects (once per hero, once per player, once per game). I’m going to merge all of them into single class with possibility to choose “visit mode”.
Note on technical part - I want to avoid huge numbers of new classes with complex inheritance so for example this stage will be represented as one object with possibility to select “mode” instead of proper OOP approach with something like “IObjectVisitor” interface and separate classes for each mode mostly due to simplicity and to avoid more load on RegisterTypes.
Requirements to visit object - some objects give different rewards depending on day of week (e.g. double bonus on day 7 in temples) or need specific resources (tree of knowledge, School of War/Magic). These should be described in a more generic way similar to seer huts (but ofc without proper “quests” part).
Think about how this data should be organized in json config. Final part, first I’d like to have such generic object implemented in code and only then move configuration to json. At this point I will also work on proper way to add map objects from mods (dwellings and representation of modding objects on map).
Extra but somewhat necessary - expose all this information to AI and make sure that AI understands usefulness of an object.
Right now I’m still around stages 1-3 with different progress among different groups of objects so there is still quite a lot to do, will upload to trunk once I’ll finish stage 4 at least for some objects, minimum plan before I’ll consider this task finished - to have all objects but events/seer huts covered by these concepts.
And there is need to not forget about sizes/passability/visitable hexes of objects.
This will be made like [0,0,0],
[0,1,1],
[0,1,X]]
or something more complex?
On a practical note, I suggest splitting CObjectHandler.cpp into CObjectHandler contain generic classes (or interface) and ObjectClasses containing specific ID-related scripts.
I also suggest creating a bucnh of seperate classes (interfaces) that handle different functions, like traits described in our wiki discussion. For example, this
Is strictly related to AI traits.
Many object classes, like CGBonusingObject, are organized in this way. Which is indeed half-assed solution, as it neither allows to split functionalities into concrete classes in elegant way nor allows to configure these ojects in general way.
Not sure what hierarchy of abstraction you propose, but I believe we need to stay with some proerties bound to subclasses. For example, CGBonusingObject may grant arbitrary bonus, but other objects will not benefit from it.
Full abstraction of map object need AI behaviour abstraction too. This means that some detail AI algorithms should be put to map object. (all AI traits are now part of map objects)
predefined AI traits (as described in wiki “Talk:”) makes AI code centred in AI, but limits addition of really new objects w/o changing AI.
Well, I already moved all new code into separate file so this should make that 7k lines file a bit smaller. I think I’ll try to reorganized this file a bit but first I’d like to finish current task.
And what about classes that can grant multiple types of objects? Chest is the simplest example with 5% chance to drop artifact instead of gold/exp. This means that it would need 3 interfaces: resources, experience and artifact. And there are more complex objects than chest
Current approach is that there should be a simple way to supply object configuration to AI so it can see all possible rewards from visiting it. However most of this belongs to latest “move config to json” part so this is still in plans.
Again - what to do with objects that grant multiple types of rewards? Create separate class for each possible combination via multiple inheritance? Right now I have ~10 types of rewards. Since in the end I want to make objects configurable via config all combinations must be supported.
In order to do this i need to either
create interface + class for each reward and then compose them into single CGObjectInstance (somewhat similar to current town/bonusing buildings relation). Don’t like it since I want to avoid maintance work (proper copying since interface + class relation would rely on pointers, mechanism to differentiate reward with multiple types from multiple separate rewards)
create class capable of handling rewards of any type, similar to current Pandora Box. This is my current approach, although a bit more organized. In the end I want to replace current ~5 classes with unified class that can handle same functionality but without any ID-dependent code.
What I have is:
class that right now I insert in the middle of inheritance tree
class CObjectWithReward : public CArmedInstance
{
std::vector<CRewardInfo> rewards;
...
};
class CGPickable : public CObjectWithReward
In the end I want to get rid of classes like CGPickable and move all necessary functionality to ObjectWithReward. For example right now Pickable have only one function - remove object once reward is granted. Instead I’d like to have “removeAfterGrant” flag in ObjectWithReward.
RewardInfo struct - it contains all possible bonuses, list is somewhat based on Pandora Box:
Right now idea is to give AI access to this struct, either directly via something like CObjectWithReward::getInfo (altough this would reveal randomized rewards e.g. AI will know content of treasure chest) or indirectly by queries “what possible rewards object of type A.B may give”. Since I’d like to replace large number of H3 objects with this mechanism completely implementing support for one object in AI shouldn’t be a problem. And since this will likey remain as a single way to configure objects till scripts we won’t have issues with supporting new objects e.g. from mods - they’ll have to use this ObjectWithRewards.
I’d say this belongs more to the reward (since you are generalizing them) than the object itself.
Given the Temple example you mentioned — on the 7th day of week it gives double morale bonus but it has also a diffrent message.
Then it might have sense for the temple to have defined both rewards (1st: {+1 morale, text 96} 2nd: {+2 morale, text 97}). And then provide some mechanism to declare which object is available on which day of week.
Though… in the temple example both rewards share the same sound. So in order not to repeat it every time, maybe the text/sound/etc properties should be defined as default on the object and then possible to overwrite in a reward.
In the current code there is one thing I really, really dislike.
As I said many times, I’d like to get to the point where confidential information does not need to be stored on every client. In particular, the actual object state (object classes instances) should not need to be present on client — its the server server that does interactions. Client just needs to know the CGObjectInstance base class members.
This rule is currently violated mainly by one thing: the getHoverText method. It is called by the player interface and virtual dispatch directs it to the actual object subclass. This requires actual object instances to be present. Moreover, the presentation logic should belong to the GUI, not the object itself. The information itself should be exposed either by the object meta-information (eg in object handler) or callback; not by formattting some string.
One of the main reasons for getHoverText to exist the showing the “visited” and “not visited”. It would be great if generalizing the visited handling could somehow help to remedy this issue.
It is not that easy, because if the actual object instance is not present, the visited information will have to be stored by someone else (player interface or client).
Providing some general class that gets information about visits and tells if object is visited in some particular context (player, active hero) would allow implementing this both in the object and on the client side.
AI should get not randomized info.
I think it could get just the object config denoting how to create the reward during the randomization. [Similarly to how hero or towns metadata are avaialble in handlers, independently from the actual object instances.]
I’m considering this. The problem is that in situations like chest when multiple rewards are available for player to select from but only one message can be displayed.
There are also some tricky cases like stables with their upgrade option (and custom message for it).
UPD: in this particular case - message that player will see is still same (messageID = 140). Text you’re referring to (descr_id 96/97) are from bonus description which is definitely part of granted bonus (and bonuses are already part of “reward”).
The problem is that even full knowledge on who visited object and when object reset its state is not enough for “can visit” checks - bonusing objects are “visited” if hero has bonus from any object of this type and some objects like shrine are visited if hero knows spell. So some access to object data seems to be necessary. And if we’re talking about “should AI visit object” topic then it give a few more considerations like handling of dwellings that were cleared this week (marking them as visited would be a logical thing to do).
So far approach like “knowledge level” looks to be more reasonable than complete removal of access to objects.
For example creature banks:
level 0 - not visible on map, no available info
1 - visible on map, player know ID/subID & object appearance (including related info like blockmap) and some fields like player-owner.
2 - visited (cleared or not) - player also knows amount of guards. Experienced players may also deduce rewards so this info can be considered public as well.
3 - owned (not applicable to banks) - full access to public data. Note that “public data” may not match full data.
It get much more complex with thieves guilds, visions spell, view earth/air spells. And I’m not sure if such “levels” can be emulated purely via some generalized approach since available information can be different between various object types. Sending objects with incomplete info (and update it when “knowledge level” changes) may be a better idea. Or it may actually overcomplicate things.
BTW - why do we have mutable hoverText property in the same time as getHoverText() method that returns hero-dependent hover text? Maybe we should at least remove that property?
Agree, I see no reason for HoverText to exist. This string should be generated dynamically when needed and not get stored. Actually, it seems to confuse serializer - hoverText causes desync in savegame.
We can solve this, providing seperate structure of actual rewards and the other structure (previously called trait) that tells AI what kind of rewards could be possibly given. The second one can be generated automatically from the first one.
I’d say that message from Treasure Chest is not a reward message. (well, except for artifact case)
The reward message is a message displayed when reward is given. In case of treasure chest it is composed from a Alternative Dialog, and then (after answer) appropriate reward is given without any message.
Well, perhaps we could have a special reward that is an alternative between two (or more rewards). Then treasure chest message is a message of “alternative reward”, while the reward components (gold, exp) come without any message. [Chest just disappears.]
Right.
I think we have here several different things that should not necesserily be unified.
The object visited state. It means whether an object will attempt giving reward when visited. It depends on the internal object state, that is modified by the fact of visits (or passing time, etc).
Whether the known reward from object is applicable to the current hero. To say this, we need both:
a) to know what the reward is (the Temple bonus is always known, the Shrine spell only after given player has visited the Shrine)
b) whether this reward works for current hero (that is if hero already knows the spell or has the bonus)
Whether it makes sense for AI (or player) to visit the object.
The 1) should be provided by some general class that can be used both by object and client to tell whether object is in “visited” state. *
2) Should be handled by player interfaces or client with using:
a) meta-data about objects (eg. if object always gives the same same bonus, it is should be known)
b) past experience (objects with randomized constant rewards, like Shrines, will need to have the reward info to be stored after first visit by a given player).
3) Belongs to the AI logic / player’s mind.
For levels 0, 1, 3 it is already working this way. [With the exception of hover text.]
As for level 2, I don’t think it should work that way. Visit gives the knowledge about parts of object state (like guards) but it is not a continuous access but more like a snapshot. Guard amount changes over time and such changes are not visible.
So I’d say that keeping info after visit belongs to player interface (or possibly to a client, since both AI/GUI might need this).
As I said, the getHoverText is evil. Initially I wanted to implement the whole thing in a “right” way and hover text was a property of object that was set by a dedicated netpack. However, it was difficult to achieve H3 behavior with this approach, so getHoverText was introduced as a temporary workaround for such cases. And it eventually dominated.
I’d say both the method and the field should be removed. Hover text should be formatted by GUI. Some additional meta-data about object type will need to be exposed.
Generally it affects all mutable fields in object. They should be moved to the GUI (player itnerface, map handler). AFAIR tracking the selected hero/town was another point causing reporting desyncs.*
Haven’t thought about this in such way, thanks
I’ll see what I’ll come with in the end (Event/Pandora reward grant code is quite complex) but this does sounds like a good approach.
[quote]
The 1) should be provided by some general class that can be used both by object and client to tell whether object is in “visited” state. *
*
Any proposition on how organize such class? Because simple class that will contain list of visited heroes won’t work as it - information on visited heroes must be available only to hero owner and object. And hero owner may change. As result - clients must keep their separate copies with different information (only recruited heroes) and server must make sure that this information is up-to-date by sending updates whenever hero owner changes.
Moving this data to hero class may be easier solution - client have full access to owned hero and object code have full access to visiting hero as well. For player-visitable this info can be moved to PlayerInfo struct which is accessible only by player himself and his allies.
And some questions.
There is one thing that bothers me in object inheritance. For example:
class CGSeerHut : public CArmedInstance
Seer Hut is an Armed Instance for the sole purpose of granting troops (and possibly - displaying exchange dialog to player). Is there any way to get rid of this? Code does needs access to some Armed Instance to properly handle exchange but maybe some temporary object will do?
We have set of classes for some of town buildings:
class CGTownBuilding : public IObjectInterface
Is it viable to either turn it into proper Object Instance or allow classes derived from this interface to use such functions like queries? (they require valid object ID)
I’m looking into possibility of applying same system to bonusing buildings in town - they’re pretty much duplicating some of adventure map objects I’m trying to replace. And right now the main limitation is that my class derives from ArmedInstance while town buildings derive from Object Interface.[/quote]
I’m not sure if we talk about the same thing here. In what I propose the class does not need to know anything about hero that exceeds the publicly available info (that is hero unique id and owner at the time of visit).
The input to the class is:
the series of visits info (like: hero with id 53 owned by Red player made visit on day 16).
the pass of time
The output from the class is, whether the particular object for given player is at the moment in a “internally visited” state. Result is one of the following: visited / not visited / unknown. The state unknown is also neeeded — eg. when we discover a Windmill from FoW, we cannot know whether it was already visited by some other player or not.
Such class would be used only for objects that are visitable once per game (like Skeleton), once per given time (Mills, Banks), once per hero (eg. Mercenary Camp) and once per player (eg. Obelisk).
The class object will be present both:
in the object instance itself — then it receives all information and is used internally by the object to determine whether reward should be given
player interface — then it receives only information known to player interface and is used by GUI to format hover text (and possibly by AI to exclude objects from visiting)
Please mind that this only a first half of the whole “how hover text should be handled” idea.
The other part is mechanism for objects that gives rewards if and only if they are applicable (ie. hero does no already own them). Such objects should have some description structure telling what rewards should be expected from them (if always known) and common code for checking whether reward is applicable for given hero instance.
Hero instance is always known (because either we are performing object logic on server or we are hero owner considering a visit).
The object state (if reward is randomized, eg Shrine) is initially not known on the client. It is however known that the object gives some randomized reward and after first visit, the player interface should remember what it received and then use this information to build hover text.
[Recognising which game state change results from visiting objects should be pretty easy — player gets notifications on visit start and end.]
Seer hut should not be an armed instance. The class is meant basically for objects with army that are meant to be fight with. It should be possible to remove the inheritance and place the CCreatureSet as a class member field. [and then use cb->giveCreatures]
Town buildings should not be objects… because they are not objects. Objects denote adventure map obhjects, with position, blockmap, and so on.
However, it should be possible allow buildings to ask queries. The object id is not the issue, it is the tracking of the visit. Query system remembers the visit start (puts CObjectVisitQuery on queries stack) and then after query is answered, it notifies the visited object.
Therefore, either CObjectVisitQuery should be generalized to handle town objects as well add some kind of “CBuildingVisitQuery”,
Or, even better… make an abstract class “CVisitQuery” that stores IObjectInterface. Then derive CObjectVisitQuery and CBuildingVisitQuery from it.
What about such situation:
Player A recruits a hero
Player A visits some hero-visitable objects with this hero
Player A loses hero (in battle or dismisses)
After some time player B recruits this hero. Client B won’t know which objects were visited by this hero - this info is known only to client A and internal state of object. Of course object still knows about heroes that have visited it but information visible to player B will be incorrect.
What to do with objects where hero may refuse visiting like Tomb or School of War? Tracking “object visited” calls won’t be enough for such situations - we’ll need something like “object actually visited”.
And that’s the source of problem. This is signature of giveCreatures:
void CGameHandler::giveCreatures(const CArmedInstance *obj, const CGHeroInstance * h, const CCreatureSet &creatures, bool remove)
1st parameter is object that gives creatures and it has to be ArmedInstance - mostly to let client create exchange dialog so hero can select desired creatures. And that dialog requires ArmedInstance.
Right now all objects that may give creatures are derived from ArmedInstance.
Good case. Then the list of visits needs to be stored in hero instance.
I take back what I said about armed instance. It is either a potential enemy or a source of army to exchange with.
Every reward-giving object should be armed instance and it is not wrong. Creating a temporary object for exchange… would be much work with no significant gains.
One case we’ll need to consider here is how reset visitors? This won’t be as trivial as erasing list of visited heroes/players.
A - keep visit date & date of latest reset and determine if object is visited based on that.
B - (probably better one) add “Reset object visitors” net pack. On applying gamestate will erase this flag from all HeroInstances/PlayerStates. We may need to optimize this somehow to avoid spamming of hundreds of such netpacks each week but other than that this approach should be OK.
And that means that it won’t be possible to apply this system to town buildings as well. At least for now
As for temporary object - perhaps implement it one way as part of garrison exchange query? Another option would be to handle this particular case (granting creatures via town building) manually - by filling free slots as much as possible discarding everything else.
[yeah, I know - ugly solution for largely optional feature]
Right now I’ve started looking into integration with AI. Can somebody who knows more about AI show desired interface for AI interaction? I can do something as simple as set of methods like “bool givesArtifacts()” but AI may need more information than that (like chance to provide artifact, chance to give some specific artifact, etc).
Currently I’m planning to move “object configuration” to handler so it will be possible to query configuration of object type without access to actual object instance. But I’m not yet sure on how AI interface of this “object configuration” should look like.
Didn’t follow the project over last week, so some points may be outdated.
I believe it would be better for AI to receive full info about all available rewards in first place and only then prepare some plan. For example, try to find all objects that can give gold or reward artifacts.
In my vision AI will preprocess all this info at the beginning of game and not poll concrete adventure objects. For that purpose, general info needs to be available independent of object instances.
Another issue are visitable objects. For example check for weekly visits is performed in isWeeklyRevisitable function on AI side. It would be better to give such info on object side.
Objects consist from one or more of “rewards”, described here: github.com/IvanSavenko/vcmi/blo … ward.h#L63
This is post-randomization state so neither AI nor human player should have access to it.
Before randomization this information must be stored in separate structure that describes how this object should be randomized. So for example given resources should be described as something like this:
And each object may contain multiple such entries (e.g. to grant different resources under different conditions - like water mill giving twice more gold on day 7).
I can give AI direct access to such structure but I’m not sure if it is AI who should read it:
Interface may be shared among different objects, and not only those that I’m trying to replace.
From what I see right now simple interface to show “type” of granted resources (and their “value”) is more that enough for AI. Of course this may change in future but this it what I see currently. So direct access to such struct will mean more coding on AI side to properly read this structure (and guess what it will contain after randomization).
Checks like these are not a problem - information like this will be available to AI so this is just a matter of replacing that check.
(although instead of “weekly revisitable” objects I have “reset duration” property so objects are not necessarily reset each week, but this is not applicable to H3 objects)
