Suggestions for town format changes

What I want to do now is this:

  1. add unnessesary “videoTavern” string to “town” data (already made locally). If it is set, then in town plays customized video (it’s for Feanor’s tavern videos - altrough they have newer smk format, so for VCMI they need to be reconverted to older BIN format understood).
    Else in tavern original video is played
  2. add unnessesary “guildBackground” to “town” data. If not set, then TPMAGE is used (not done yet)
  3. if “silo” set to "gold’, I want change it to +500 gold income (like in HMM2 for statues) instead of +1 gold
  4. I want to add unnecessary “boat” section to “faction”. If “animation” is set, faction will buy ships with new DEFs. I put discussion in WIKI (for faction). Now I only see how to add new DEFs, as about bonuses, I must study sources deeper.

I’ve moved this post from “Git guidelines” thread. Keep it on topic. Ivan.

First - word you’re looking for is “optional”, not “unnessesary” (Rus: “не обязательный” vs “не нужный”).

As for suggestions:
1 & 2) I don’t like optional fields that default to H3 files (read - to hardcoded value, as opposed to empty string/zero). Instead we should either:
a - Mark this field as required in schema but for compatibility set it to H3 value if not present (till next release).
b - Add config for “Default town”. Right now we have several properties that were shared across towns in H3 duplicated in every town (e.g. generic building lines like Town Hall/Castle). Instead we can add config where all such default settings will reside and merge it into town-specific config to fill all missing data. I like this approach since it will match H3 better.

  1. This field have also effect at least on starting bonus. You’ll need to either handle this case as well or split this field into two: starting resource and silo bonus. And as for silo bonus - I’d rather have it in form of proper resource set instead of hardcoding resource quantities. Like this: “siloBonus” : { “wood” : 5, “gold” : 1000 }.

  2. Animation for boats is not as simple as it sounds. IIRC each ship have their own set of flags and water movement animations. And all of them are hardcoded to current 3 ships. If you want to implement this you’ll have to fix this and replace with something more flexible.
    However I’d rather wait with this for a while - I have some plans to make adventure map objects a bit more flexible, including better way to add new objects.

I’ll think about 1 & 2 -> a.
It’s good idea to handle it at stage of loading JSONs.

I now succeeded in 1 & 2.
For use of Feanor tavern videos I replaced SMACKW32.DLL, BINKW32.DLL from Hmm3 by his more recent versions.
If I’ll have time tomorrow, maybe I’ll make a variant into life. I want to test commit to GIT:-)

PS is there is a function, that gives access to creature properties by it’s ID?
I didn’t searched too hard.

As for “Like this: “siloBonus” : { “wood” : 5, “gold” : 1000 }.”, this is also good. But I think it can be added to any “building” in “buildings” section (maybe except dwellings of creautures (30+). And it’s better to add this to VillageHall/…/Capitol

“b” also happens during json loading:

  1. load default town config
  2. load current town config
  3. merge them
  4. load town from json

Besides - this will also allow us to reduce copy-pasting that we have right now in town config - quite a lot of fields like generic buildings are identical across all towns (and most of town from mods).

Only via CCreature object:

const CCreature * creature = VLC->creh->creatures[creatureID];
creature->getAttack();

Sounds OK. Except that I’d rather see it in separate structure that in future can be used for the rest of town buildings bonuses:

"resourceSilo" :
{
    "name" : "...",
    "description" : "...",
    "produces" : {
        "resources" : { ... }
    }
}

And as for Village Hall/Capitol line - that’s one more reason to use approach “b” for 1&2. This will also reduce amount of maintance work for mods - because if you’ll do this all modders that have town with “generic” town hall will have to sync their town config with ours. With approach “b” they can just throw away that part of config and don’t worry about keeping it in sync or such things like localization.

Throughts about war machines realization

I had throught about current hard coded war machines, and here are my ideas of making them extendable.

Now in «warMachine» field of «town» we write creature’s name «ballista»/ «firstAid» etc.
Then program returns corresponding «ballista»/… artifact ID.

I suggest to use in «warMachine» Artifact name «ballista»/etc or any new artifact.

Artifact will have all needed information on war machine creature through bonus system, like:

        "catapult":
     {
               "index" : 3,
            "type" : "HERO"],
            "bonuses":
               {
                     {
                               "type" : "SIEGE_MACHINE",
                               "subtype" : "creature.catapult"
                     }

               }

       },
      "ballista":
     {
               "index" : 4,
            "type" : "HERO"],
            "bonuses":
               {
                     {
                               "type" : "SIEGE_MACHINE",
                               "subtype" : "creature.ballista"
                     }

       },
      "ammoCart":
     {
               "index" : 5,
            "type" : "HERO"],
            "bonuses":
               {
                     {
                               "type" : "SIEGE_MACHINE",
                               "subtype" : "creature.ammoCart"
                     }

       },
      "firstAidTent":
 {
               "index" : 6,
            "type" : "HERO"],
            "bonuses":
               {
                     {
                               "type" : "SIEGE_MACHINE",
                               "subtype" : "creature.firstAidTent"
                     }

       },

When battle starts, VCMI will check, are there artefacts in MACH1, MACH2, MACH3, MACH4 (MACH1 is for «ballista» now, MACH2 for ammoCart, MACH3 for tent, MACH4 for catapult).
If MACH1/2/3/4 artefacts are found, VCMI will check if Bonus::SIEGE_MACHINE is among bonuses. If it is along bonuses, than on start of battle VCMI will place creature from «subtype» of artefact MACH1 to place of ballista, and so on.

That’s all. It looks simple. And doesn’t destroy logic behind war machines.
If hero is under siege, creature from MACH1, MACH4 will not be placed on field.

The only left question is with catapult. Catapult is always with hero. So i think there new field «catapult» must be added to hero class JSON.
And according to artefact written there catapult will be functioning (stats, abilities and etc will be taken from creature JSON).
If someone will make catapult unit not shooting, it will be his fault, but the game will use whatever he put in.

PS sorry if post formatting is bad. I’m writing not from home, so don’t have password from my account here. And can’t correct it.

  • I really don’t like relying on slot numbers. Just remove this part and place siege machines from all active bonuses.

  • How will vcmi determine a hex in which war machine will be placed? Probably you’ll need to specify row in which machine should be placed (additional info field I guess).

  • How secondary skills should react? Right now they won’t work with new war machines.

  • You need to recheck every place in our code where we have hardcoded checks for war machines. For example ballista is not present for defender in siege. Catapult projectiles follow parabola during flight - custom war machines may want something different (cannon for example).

I believe it would be nice to assume new war machines replace old ones - use same slots in backpack and on battlefield. This way we could unify some logic, at least.

Correct me if I’m wrong but this won’t fix for example skills - they give bonus to creatures, not to artifact in specific slot so if somebody will replace a war machine then new one still won’t use ballistics/first aid/artillery.

And this will also remove possibility of extra war machines. Yes, right now it is impossible to add extra slot for some fifth war machine but I don’t see any reason to block possibility of misc artifact that have presence on a battlefield.

Possible solution to this one - consider specific tiles to be locked during siege and don’t place war machines on them.

If bonus for artefacts/creatures will be made, that give additional creatures on battlefield, there will be no need for this additional war machines on field. It will be made through “special”:“true” creatures, with SIEGE_WEAPON bonus.

Correction of Ballistics is no question, Ballistics will give bonuses to creautres with CATAPULT bonus.
Artillery may be made to give bonus to creatures with SIEGE_WEAPON and SHOOTER bonus.
First Aid will give bonuses to creatures with HEALER ability.

Or extend bonuses of creatures, add to NON_LIVING, UNDEAD, etc. new types BALLISTA, FIRST_AID_TENT, AMMO_CART (we already have CATAPULT).

Something like this may work for skills I guess. Assuming that bonus system can work like that. And don’t forget that skill should also give player control over object.

As for additional creatures - there is difference between “artifact that gives creatures” and “war machine” - latter one disappears if destroyed on battlefield.

Perhaps describe these bonuses as something like this?
“type” : “EXTRA_CREATURES” for creatures at start of each battle and something like “BATTLE_AVATAR” for cases where summoned creature represents an artifact, if creature killed -> artifact destroyed.
“subtype” : creature ID to summon
"val" : number of creatures to give
"addInfo" : optional, row in which to place creature, if 0 or not present - place as normal summon.

This will also cover recreating catapult - it can be handled as “EXTRA_CREATURES” and won’t be destroyed if killed in battle, while the rest of machines will be removed on destruction.

How about possibility of assigning ‘standard’ building functionalities to new buildings from mods?

I now think about it like
"building17":{
“specialBuilding”:
{
“type”:“COVER_OF_DARKNESS”,
//“subtype”:“creature.skeleton”, - not available for this bonus but stay in schema
"addInfo": 100 - some reserved info
"val":15 //size of darkness
}

“building21”:{
“specialBuilding”:
{

“type”:“SKELETON_TRANSFORMER”,
“subtype”:“creature.demon”, - converts to demons
//“addInfo”: 100 - some reserved info
//“val”:15
}
This will help to reuse standard bonuses with possibility to change values, like instead of attack skill give defence skill points.
This goes especially for markets.

This can be made incrementally, like first realize 2-3 bonuses, then next (they are in different places). This will help to cover lack of special buildings functioning for mods.
And standard towns will also be made to this schema by changing JSONs.

I think I could work on it tomorrow, if this idea is accepted.

PS About War Machines artefacts. I saw that war machine artefacts are assigned to bigArtefacts group. So “type”:“BIG_ARTEFACT” can say that this is war machine artefact, and it cannot be placed to backpack.

For now - let’s wait. For (relatively) simple objects like Stables/Hall of Valhalla I’d like to use “configurable objects” idea. For more complex like market-related we may need another approach - these objects are too complex to be described by bonuses. They need list of available trade items, how they should be randomized on reset, how often they will be reset, how their cost will be calculated.

Yes, I worked on it a little and think, that I don’t yet have skills to make it complex.
I throught out about buildings. I think, for first stage of making buildings modded we may use next:

“building2”:
{
“buildingBonuses”:
{
“buildingType”:“SKELETON_TRANSFORMER”
}
}

To class CBuilding add some new data for compatibility:
BuildingID BuildingType;
int stdTownType;

so when parser will see "“buildingType”:“SKELETON_TRANSFORMER”, it will do

BuildingType=BuildingID::SKELETON_TRANSFORMER;
stdTownType=ETownType::NECROPOLIS;

So all places where building is checked to be built,be corrected, like

addBonusIfBuilt(BuildingID::TAVERN, Bonus::MORALE, +1);

to something working with BuildingType, stdTownType, not with BuildingID.
When all buildings types are made to JSON (including CASTLE, MAGE_GUILD etc), than variety of configuring given bonuses can be made. Like instead of Bonus::MORALE, +1 to give Bonus::LUCK, +2 or something like this. So markets, mage guilds, dwellings would not require additional bonuses, working only from “buildingType”, while others need to be additionally changed.

There goes Ivan’s idea to make default town JSON and to merge it with mods, so modders will not require to change their mods everytime something is added.
This building type system can drastically change functioning of buildings (it’s not based on IDs), so we need default json to let them work with their previously made mods, if we go into buildings modding.

PS Will try to mess with war machines. I think for first time war machine position on battlefield will be connected to artefact slot (MACH…).
Artefact bonus saying it’s WAR_MACHINE will be

{
"type":"WAR_MACHINE",
"subtype":"creature.creature",
"addInfo":1 //0 - machine is immortal, 1 - machine is killed after battle
}

Also this type will say VCMI to add this artefact to bigArtefacts, that are not placed to backpack.
Will look if I could make this into code.

Bonuses from ballistics/artillery/first aid will be realized later. For them we need new creature types for limiters. When secondary skills be made modded, it will look in them like “limiters”: “HEALER”/“BALLISTA”/“CATAPULT”/“CART”]. They will live for some time without bonuses (let’s current bonuses only for “creature.catapult”/etc. remain for some time.

On buildings - as I said it is better to wait until I’m done with configurable buildings. After that we make take a look on what features are still missing.
And no to ugly workarounds like changing town/building type.

In general I think we can split buildings into several groups:

  1. Generic functions, e.g. town hall, mages guild, fort, dwellings. Can be easily moved to config to allow tweaking of values.
  2. Simple bonuses, mostly to defender (tavern, stormclouds). Can be moved to config.
  3. Hero visitable - those that can be visited by hero. Will be covered by configurable objects
  4. Trade - I guess we may look in how to configure this, but this is more complex than it looks.
  5. Unique functionality - Inferno Grail is really good example. These are too unique to be useful to remove hardcode or to provide configuration. At most - move it to scripting once it will be available.

I suppose I can make list that will include all buildings & group they belong to. I would need to demangle town buildings functions anyway.

How it will look in config:

  1. unique entries in config, e.g. “givesSpells” : 5, 0, 0, 0, 0 ] // gives 5 spells to lvl1
  2. list of bonuses + propagator(?) - town, player, alliance or global.
  3. as rest of configurable buildings, still WIP though.
  4. not configurable, at least for now.
  5. list of hardcoded functions, perhaps we can even merge trade in it for now: “extraFeatures” : “infernoGrail”, “portalOfSummon”]

Maybe rename this bonus to something else? Because nothing in this description is war machine-only. What makes war machine actual war machine is bonus of a creature. Something like “GIVES_CREATURES” would be more logical.

I’d rather turn this into a normal property of an artifact. One that can be changed separately by modders.

Oh - and please, make this as a separate branch on github so it can be committed & reviewed separately from rest of your changes. This is how to do this via console, your UI should have commands with same name:
git checkout feature/warmachines – switch to specific branch, will be created if needed

git commit – commits changes you’ve made to active branch

When you’re done push this to github:
git push warmachines – upload code to remote repo, e.g. github

I made separate branch.
Seems it will be not in near future:-)

War Machines are distinguished from GIVES_CREATURES by the way of working. They cannot move and occupy fixed hexes on battle field. So it’s not good to make them freely placed or go to backpack. I will use artifact slot positions to determine place on battle map.

But this is property of a creature, not artifact. What makes creature immovable is 0 speed and special bonus (SIEGE_WEAPON I think). Properties of artifact have no effect on this.

Artifact bonuses are responsible for placing creature object in battle.
Creature bonuses are responsible for how this creature acts in battle.

Why? Moving war machine (or any other artifact) to backpack will disable it and it won’t have any effect in battles.

And modder may already enable placing it into any other slot - big artifact property only blocks moving to backpack. It won’t block placing artifact into another slot (possibly not related to war machines)

Yes, it’so. Still how can VCMI decide, where to place creature? There are 4 fixed places on battlefield where war machine will be placed.
I throught that depending on artefact slot (MACH1-MACH4) slot on field will be selected for compatibility.
But from Artefact Handler i couldn’t access some creature properties… So you think i must not bother with slots and only make WAR_MACHINE artefacts big?
Then creature placement in war machine slots (for AMMO_CART/TENT/CATAPULT/BALLISTA) must be pointed in bonus property (like “addInfo”:52 - position on field)

Currently war machines placed on battlefield like

handleWarMachine(0, ArtifactPosition::MACH1, CreatureID::BALLISTA, 52);
handleWarMachine(0, ArtifactPosition::MACH2, CreatureID::AMMO_CART, 18);

If make free slot chosing for war machines, that it will become very difficult to handle destruction of war machine. Because now after battle from creatureID gets ArtifactID. So if one creature is assigned to two artifacts, will be selected first available. Same for placing creatures on battle field. Slot is selected, and then corresponding machine from art in slot is placed to field.
If make free slots selection, there can be artifacts using same positions on field.
Artifact is not connected with creature in both ways, that’s problem. Creature immortality is also will be placed in artefact bonus only.

war machines slots are used only for machines now. If we disconnect logic from slot, they will be not used most of the time. And again, we can’t get artefact from creature, so killing creature and removing it’s art will become problem.

OK, i’ll use next format of bonus than:
{
“type”:“WAR_MACHINE”,
“subtype”:“creature.megaSuperMachine”,
“val”:52, //used place on battlefield.
“addInfo”:0 //0 - killed after battle, artefact is removed, 1 - resurrected like catapult
}

Ballistics, First Aid Tent, Artillery will become not working after that, because if slots are free, there no possibility to choose creature to give bonus.
I can imagine next: check, if creature has bonus CATAPULT and SIEGE_MACHINE, than give it bonuses of Ballistics.
I can imagine next: check, if creature has bonus SHOOTER and SIEGE_MACHINE, but not CATAPULT, than give it bonuses of Artillery.
I can imagine next: check, if creature has bonus HEALER and SIEGE_MACHINE, than give it bonuses of First Aid.

First artefact found for battle position coded in val will be used in battle. So addtitonal logic to store found war machine locations will be required. Because there can be many.

And what about catapult? It cannot be moved from slot. I must check, if it’s creature has CATAPULT ability, and ban it from moving? Than catapult will be exeption - it still will be required to be in MACH4 slot.
Than VCMI will go from classic Heroes 3 because war machines will be handled differently from classics.

How to determine placement of creature in battle?
a) Simple version - based on slot in which artifact is placed.
b) Based on field in GIVES_CREATURES select row in which creature should be placed. Not tile but row - because attacker/defender have different positions for war machines.

How to determine artifact-source of creature?
For example - after creating stack for battle give it bonus “Comes from artifact” with value set to slot ID where artifact is located. There may be other solutions but this is first one I found.
One possible loophole - if bonus is not coming from artifact equipped by hero then it won’t work. But this is not applicable to H3 anyway.

Removing big artifact flag or changing slot assignment would break mechanics
No. What I’m proposing is to separate big artifact flag from war machines bonus, not remove it.
So by default, without mods, war machines would have “big artifact” flag set implicitly AND will have only one slot in which they can be placed. So player won’t be able to move artifact to another slot or to backpack.

Yes it will be possible to change this via mods or by editing configs but default behaviour would be correct.

Moveable catapult
IIRC right now we’re checking for catapult artifact ID. This should be replaced with property like “immovable” and given to both catapult and spellbook.