Json configuration for towns

Hi. I just created page on wiki with a proposed configuration for towns. It basic and still needs merging of existing configs. Most of this missing parts are mentioned on wiki.
Some notes regarding editing:

  • Renaming fields: please do. Most of current names are copy-pasted from existing configs.

  • Changes in buildings\structures fields: changes there may need a lot of rewrites in code - ask me before making any of them.

  • Numeric ID’s: should remain there at least until most of hardcoded buildings mechanics will be removed. Maybe buildings may use same bonus system as the rest of the game? Certainly needs discussion.

Huh, new towns already? I thought it’s a distant future :stuck_out_tongue:

Commanders are not a part of town as such, but rather a part of faction. We probably will have to specify separate structure, even simple, to describe faction. Not only commanders, but also related heroes, dwellings, native terrain, alignment, ship (!) etc. Just to keep things together and let modders refer to faction as a whole

“Town” - won’t be hard to implement with exception for special buildings (hardcoded). But full “factions” that’s definitely distant future. So no Cove for now :slight_smile:
My first priority here however is create mod-friendly loading process:

  1. load multiple configs (right now I have two of them: h3 and vcmi)
  2. merge them
  3. feed resulting structure to TownHandler
    And do this allowing any number of loaded objects.

After this it will be possible to give some real functionality to ModHandler, think about how mods will look like, etc.

Yeah - I mentioned that on wiki. Possibly even town mechanics and gui will be separate. There is just too much data needed for towns - some split is necessary. And we may give separate (unplayable) faction for neutrals if needed.

The best place place for commanders however should be hero class I think - closer to heroes and will allow different commanders for might and magic classes. But this is just idea for future.

You forgot about one thing, name and description for building.
Without change in code there is no option to add names for new building for city. If i know u rebuild config to will not use original files .txt.

Buildings and structures are a bit messy right now. I’ll add names and descriptions when I’ll find better way to organize this.

Some kind of status update.
Second part committed, puzzle map and sieges now are probably the biggest missing pieces.

What I’m planning to do next:
a) Split huge buldings.json into one file per town (towns/castle.json, towns/rampart.json, …). Not yet sure where factions will be placed.
b) Create CFactionHandler - probably better to store factions separately from towns to avoid unnecessary dependencies between handlers.

After this I’ll try to implementing town buildings configuration using bonus system: Don’t think that it will work “as is” for towns - even bonusing buildings have strong separation between effects:

  • constant effects with different range (visiting hero, player, alliance)
  • once per week
  • once per hero
    So unless there is way to make this work with bonus system I am going with something like this:
"functions" : //field in building description
    "someString" :
        "type" : "oncePerWeek",
        "totalUses" : 2, //gives bonus to two heroes per week
        // extra fields like reset time instead of hardcoded 7 days?
        "value" :  ... ]  //bonuses described using bonus format
    "oneMoreString" :
        "type" : "allowsRecruit",
        "creatures" :  "someCreature" ] // for dwellings, bonuses won't be used here

This won’t cover ALL buildings - some of them (like Castle Gate or Portal of Summoning) will be left hardcoded until scripting.

And one more thing:
Currently there is two different naming formats on wiki:
“camelCase”, starts with lower case - used for config fields
"CamelCase", starts with upper case - used for identifiers

This is a bit confusing especially in such cases like terrains or resources. I think we should use only one format: “camelCase” - right now it is used more often than “CamelCase”.

I would say it is a good thing to have both if they are always done right. If the start is small then the name comes from the schema. If it is big then the name is user defined and refers to an object. Terrains and resources should become user defined at some point (we already prepared for resources in the creatures format).

There are also the names of bonuses that are BONUS_NAME. I would change them to BonusName because they would become user defined objects when scripting is implemented. And I think BonusName is nicer to read and write than BONUS_NAME.

I don’t think that user should bother with "is this entry comes from schema or user-defined?"
Mods for new terrains or new resources will be quite rare compared to new creatures\new towns so some of modders may consider them as part of schema.

What if there will be option to add user-defined entries into config? Good scripting support may allow this. I have no doubts that this will be requested at some point.

As for bonuses - they’re are coming from format we’re using in code for constants. List of strings is generated using macro so it has same format as enumeration in source code.
B(b)onusName -> BONUS_NAME conversion is simple one to implement.

What format we should use for bonuses or any other enumerations if there will be way to add them via scripting? Schema can define enumerations too so both formats will be applicable to enums.

What did you do with terrain costs? Two-dimensional table got collapsed to one dimension… IIRC the cost is determined by both faction and type of terrain, that’s why it’s 2D.

Movement speed (without roads) is determined only by terrain type.
BUT if all creatures in army are on their native terrain movement costs are ignored.
For neutrals any terrain considered native.

If you’ll take a look on old terrains.json you’ll notice that each faction have only one “fast” terrain (100 speed) and it is always same as native.

Right now vcmi checks for hero faction instead and correct mechanics marked as “todo”. Thanks for reminder - I’ll implement this in next commit.

Looks like you’re correct. I didn’t checked that all costs for all factions are identical (except native terrain of course). It’s strange that it was separated.

Not exactly sure what you mean by that, that is what exact buildings working you’d like to realize by bonus system.
I think the current BS (bonus system) usage extent should NOT be extended. Bonus system is intended mostly for effects that affects a range of nodes (like morale bonus going from town to hero and its children-nodes: stacks). It can be also used for effects that are expected to disappear after some time (though that part of BS has some evil, subtle bugs) — eg. movement premy from stables or “till the end of week” bonuses.

However it would be a good idea to get rid of CGTownInstance::recreateBuildingsBonuses by turning buildings into bonus ystem nodes and putting the bonuses into them. Then link the building node with town node once the building is constructed and let the bonus naturally flow.
If you need any bonus system know-how, I’ll be happy to help.

What is the rationale behind separating town and faction? I never really distinguished them and always thought of creature faction as town type. The only odd-case is neutral faction that hardly can be called a town… is there something more?

regarding BS:

  • such effects like resource generation should go to bonus system instead of current CGTownInstance::getIncome and separate resource silo code.
  • stables are already part of BS but should be configurable. Mana vortex may remain hardcoded unless there is way to integrate it into BS.
  • everything from recreateTownBonuses() should be moved to config too.
  • some generic structures like hordes may be part of BS (growth bonus is already supported, other buildings may need some new bonuses).

Evaluating amount of markets and thieves guilds may be done via BS.
Trade options (resource-resource, resource-artifact, etc) should be made configurable but NOT as part of bonus system. Same goes to dwellings.

Introducing new bonuses only to implement some tricky buildings like dwarven treasury is definitely a bad idea.

  • Loading towns would need loaded creatures and vice versa. Factions can be used to break this loop. Several other areas have this circular connection as well. Loading can become more complex with string ID’s from mods so this is cheap way to remove some complexity
  • Moving some data away from town configuration - there is just too much of it. IMO town should store only buildings-related data (+gui).
  • Neutrals faction can be used to get rid of “invalid town” checks and gives location to store data specific to neutrals.

I once wanted to do that… but the bonus system works in the exactly opposite way. We want income to be inherited by player from owned object. In BS player node doesn’t know that some hero (by specialty or stack) or town has some “income” bonus. It is hero and its stack that get bonuses from player.
A workaround that comes to mind is using propagators mechanism so the bonus are exported from their sources onto the owning players node. But
a) Asking about income of any other node would give meaningless results (everything sees only global income)
b) that won’t work because the way that propagators are implemented — even if there is more than path between source and destination nodes, the bonus is propagated only once. So if you’d have two towns with City Hall (building being bonus node with propagated income bonus), you’d still receive income as from one.

[Same goes for marketplaces and thieves guilds]

I planned extending bonus system to support also that inverted bonus inheritance model (so called “reverse propagation” that was planned for “sixth part”) but never actually got into that. Bonus system is quite complex and I don’t like touching it (and making it even bigger).
On the other hand I see that bonus system already has all the information needed to make that (income system and so) work and current approach is not nice. But it would be difficult to improve it, I’d leave it as is for now.

I’m not sure… but sounds fair enough.

Btw, creatures and towns obviously need to be able to refer to each other but does it really require that they’re loaded before each other? Why can’t just town be loaded with some string id of creature? I think we shouldn’t need to have game object loaded to be able to refer to it with id.

It looks like we have bugs anyway - IIRC Lighthouses in Castle are cumulative. So we need some way to give bonuses for each building as well.
For now check for each object can be done separately in - this is how resource bonuses from hero arts works right now. But this certainly will be limited to small set of bonuses.

Built buildings ID’s is clearly an implementation detail which shouldn’t be exposed too much to the rest of engine. If BS won’t work in some areas like income then this should be described with alternate system or with scripting for most complex cases.
Reason behind choosing BS is that it can describe almost any possible bonus - no need to write more-less same code twice.

And keep that string ID for the whole game? In this case it is possible.
Otherwise loading becomes more complex - there has to be two stages where first one creates entries in handlers for all available objects and second one that resolves string ID’s into numeric ID’s or pointers. Does not sounds like good idea for me - presence of semi-loaded data between these stages may cause issues.
And so far idea was to keep numeric ID’s in but don’t expose them to modders.

We already have same issues with loading inside one handler (building dependencies, creature upgrades). I’d rather avoid similar issues between different handlers as well - this may cause more problems than it will solve.

AFAIR it works now. Buildings are not nodes — towns receives bonuses depending on structures it has built in recreateBuildingsBonuses method. Each town has its own, unique bonus instance and they’re cumulative therefore.

The reasonable improvement ATM may be to just create config file with some kind of map<Building, vector>. Then keep recreateBuildingsBonuses() but made it entirely dependant on the config file. Consider the following:

//assuming building ID is some kind of pair<TownID, BuildingTypeID>
map<BuildingID, vector<Bonus>> buildingsBonuses; //in some handler 

void CGTownInstance::recreateBuildingsBonuses()
	//clear old bonuses
	FOREACH(BuildingID building, this->builtBuildings)
		FOREACH(Bonus bonus, buildingsBonuses[building])
			this->addNewBonus(new Bonus(bonus));

That way it would be configurable and can be implemented right now without changing BS.
When BS is extended to support needed model of accumulating, it will be easy to just create Building node with bonuses from the very same config file.

Income just doesn’t make much sense in terms of “bonus inheritance” that is main feature of bonus system. It would be using it against its design.
But… that reverse income lookup can be pretty simple to implement as a separate special-purpose function in bonus node. That won’t be elegant and will hove some shortcomings… but it may be better than writing separate system and duplicating bonus system code (duplicating is worst evil).

If you are fine with limited functionality (only simple adding values of GENERATE_RESOURCE bonuses, no percent multipliers and so) I can take my try in implementing this.

We could have CreatureID class that is created from string id but has semantics of smart pointer to CCreature. [or, generally some template for that, but that’s implementation detail] Then we can have references to creatures without loading them first, one stage loading and pointer semantics. For internal workings it may replace numeric IDs pretty well if accept additional overhead (string instead of int id -> memory cost + lookup time).

Or use numeric IDs but allocate them as soon as given string id is encountered in any config towns. Then we reading creature id will be: get string id, check if it’s on the list — if yes - return serial position; if no — add to the list and give numeric id.
The drawback is that numeric IDs will be totally dependent on the loading order and contents of config files. This won’t be a problem only if numeric IDs are an implementation detail of engine that is not exposed for modders.

Hmm… I have to many alternative ideas. I guess your proposition was just simpler in terms of implementation.

Do as you consider appropriate. :slight_smile:

Why bonus recreation is needed at the first place? Add required bonuses on game start + create new ones when something is built in town. This method is not used anywhere else.

Anyway this sounds good for me. Right now I am mostly interested in receiving more-less fixed config format instead of one that changes with each release. Internal buildings nodes can be implemented later - I don’t see any real need in them right now.

But some bits of separate system should be done anyway. Apart from buildings with constant effect we have buildings that trigger on hero visit, trade buildings, dwellings.
I’ll make list on how each building should be implemented IMO (BS, not BS, hardcode till scripting). I would need it anyway and you can say if something from it should be moved in\from BS. :wink:

As for ID’s:
I am almost sure that scripting support would need notable amount of changes to loading anyway (e.g. ID conflicts resolving). Implementing some complex system right now only to remove it later does not seems right.

I still think that handlers should be initialized one by one without circular dependencies. But something like this can be quite useful to get rid of load order inside handlers (dependences\upgrades).
What I am thinking is some <stringID, callback> pair. Callback is triggered when stringID loaded, or immediately if it was loaded before. Should work fine for current needs.

Oh - and one more thing. What is the reason behind saving towns inside save games? Everything is completely constant there. Just precaution or there is something I don’t know about?
Original code I am talking about:

template <typename Handler> void serialize(Handler &h, const int version)
	h & towns;   //  why towns are serialized during saving or loading
		loadStructures(); // but structures are only recreated on loading?

It is not really needed. You could add bonus when building is constructed and remove when it’s demolished. And handle some corner cases like Tavern/Brotherhood of Sword *]. But it is easier to have one function that makes it all and the time overhead is irrelevant.

*] The bonus from tavern should not appear when Brotherhood of Sword is built. Or we could introduce DoesntHaveAnotherBonusLimiter. :wink:

Obviously. :wink:
I don’t see though why the system has to be complex. [Though now it may not be the time for it.] Using string as ID should be pretty simple. Later, when mods come, the string ID will have to be prefixed with mod id (or both ids stored as pair, irrelevant) but that should not affect strongly neither config files nor the loading code. Loader just will need to know what mod it is loading and automatically add its id to any unqualified id it encounters.

Storing just string id allows to initialize handlers in any order, independently from each other. That seems even better.
It is natural for me that I say:
a) Pikeman belongs to Castle
b) Castle has Pikeman as its first-level creature
Introducing a faction concept to break the circle of mutual dependency seems forced to me.

But again — just saying ideas, you decide what is reasonable to use and what not. Don’t feel constrained. :slight_smile:

Uhm… I’m not sure how would it be used. When unloaded ID is encountered, the callback is registered to resolve it as soon as it is loaded and put actual pointer?

Precaution. CTown contains mechanics-relevant data. If player saved a game, modified game config, and load, that would lead to a possibly inconsistent game state. Basically we want ALL mechanics-relevant data to be saved with game. If some game rules are to be changed, engine has to be aware of that.

Structures were recreated on loading because they contained GUI information (defnames, groups and so) that can be safely changed between saving and loading game. More like it was rather some save-space optimization than conscious design decision. Similarly, most handlers are saved but general text handler is not serialized. (so language pack could affect savegame)

I don’t think that this will be needed. I already implemented “building A upgrades B” relation. Right now it is used only for town screen - upgrades usually replace graphics of base. This should work for such buildings like brotherhood as well.

Haven’t thought about that. This is definitely better :slight_smile:

Yeah. Callback will look like this, with newly loaded pointer as the only paramenter:
function<void(CCreature *)> callback;

During upgrades loading string -> pointer conversion will be scheduled:

callback = ](CCreature * newCreature)

foreach(upgradeID, node"upgrades"])
    handler->onCreatureLoaded(upgradeID,  callback);

And when loading is done any non-triggered callbacks will indicate missing\incorrect ID’s.

Fair point. I don’t feel like merging them though - neutrals deserve some place to live in :slight_smile:
Leaving it as it - CTownHander which stores factions and towns should be OK - it won’t cause any issues anyway.

I did this.
Town. Number. Ten.
Extremely buggy so no package for testing right now.