Suggestions for town format changes

[Macron1]

" This should be replaced with property like “immovable” "

By the way, this will provide possibility to create “cursed” artefacts, which couln’t be removed any way except taken by enemy hero defeated yours.

“Based on field in GIVES_CREATURES select row in which creature should be placed”

This bonus is already working?
I seen no traces of it in code. Or it must still be added?
Than maybe there is no reason to create WAR_MACHINE bonus, but use GIVES_MACHINES with creature with bonus SIEGE_WEAPON or how it was called? If siege weapon, place in needed row in reserved for war machines are.

[Ivan]

I seen no traces of it in code. Or it must still be added?

It must be added.

Oh - and there is another missing piece:

By the way, this will provide possibility to create “cursed” artefacts, which couln’t be removed any way except taken by enemy hero defeated yours.

War machines and spellbook can not be captured in battle. This situation needs handling as well.

As usual - I advice to run a search in vcmi code for any other hardcoded elements related to war machines. (this is why some “easy” features are not implemented - too many hardcoded details).

[Macron1]

As usual - I advice to run a search in vcmi code for any other hardcoded elements related to war machines. (this is why some “easy” features are not implemented - too many hardcoded details).

Yes, there are lot of hardcoded features.
Simpliest way is to leave them, only adding new functionality and merging this. But it will oppose idea to get all features to mods.

I throught by the way of another way of handling war machines - using “faction”. In faction we can provide info on artefact, creature, and any other information we have (bonus information structure is not suited to configure all aspects and to access it from any part of code). But it will be not elegant, I think.

[Macron1]

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.

I’m still thinking, that idea to move part of standard/hardcoded buildings functions to json is a good idea.
Cause it’s building, it must have only one function at a time, with configurable parameters.
This configurable section can have “Bonus” json format.

"building5":
{

"function":
{
"type":"CASTLE",
"subtype":"",   //subtype - for market, for example - CREATURES_RESOURCES - subtype of market
"val":60
"base":"PERCENT_TO_ALL"   //growth bonus for castle
}

}

It can save current logic and allow modding changes in logic of town also. But it’s a lot of work since every building type must be unhardcoded.

[Ivan]

The problem with such “configurable parameters” is that it would be quite hard to describe most of buildings functions with 4 numbers (type/subtype/value/addInfo). Something like this sounds much better from my point of view:
wiki.vcmi.eu/index.php?title=Building_bonuses

How would you describe Library? How to differentiate permanent bonus to visitor (e.g. Order of Fire) from global bonuses to all heroes (e.g. Colossus). What about more complicate buildings like trade-related? And don’t forget that you also need to keep this readable.

IMO except for specific cases like Colossus we should not use bonus system for town buildings.

<sorry, won’t write more - situation in country had greatly demoralized me >

[Macron1]

<sorry, won’t write more - situation in country had greatly demoralized me >

I undestand you. I am also demoralized.

Something like this sounds much better from my point of view:

I suggested almost the same thing earlier:

"function" { "type":<hardcoded special building string>,
"subtype":
"val":
}

The only difference that with subtype i can change hardcoded functionality, like

“type”:“cover_of_darkness”,
“val”:6 //radius of darkness

Or

“type”:“god of fire”,
“subtype”:“creature.robocop”,
“val”:15 //bonus to growth

“type”: “wall_of_knowledge”,
“subtype”:“primarySkill.attack”,
“val”:3

This way we can get new buildings without bonus system, using only hardcoded functionality.

Or

“type”: “CASTLE”,
“val”:70 //growth in percent to creatures in town

It’s a long work, but this is easy to implement and to incorporate into hardcoded logic not BuildingID, but building->type field.

PS I want to start with boats tomorrow.
I want to add instead of getBoatType string std::string getBoat() with names of DEFs (AB00_.def etc).
For objects it will return standard 3 def names of ships.
For Town/Hero Instance it will take boat animnation path from “faction”.

In faction it will look like

"boat":{
"animation":"newCoolBoat"
}

Later this section will be extended with cost of ship, and with special abilities like “FLYING”:true, “movementBonus”:200 etc.
I looked through sources, adding new boat animation can be made easy, as i estimate.

[Ivan]

I suggested almost the same thing earlier:

It is not “almost same”.
Again - how would you describe Library that gives one spell to each level? How would you describe Colossus AND Necromancy Amplifier (effect of first one should not stack, unlike second).

I don’t think that having something that looks like a bonus system but is not a bonus system is a good idea - it will only confuse both modders and developers.

Instead I suggest to turn each building function in a separate property of a CBuilding structure.

What I’m OK with:

"building" : {
    "cost" : { ... },
    "produces" : { ... },
    "givesSpells" :  5, 4, 3, 2, 1 ]
     ...
}

What I don’t want to have:

"building5": 
{
  "function": 
  { 
    "type":"CASTLE", 
    "subtype":"",   //subtype - for market, for example - CREATURES_RESOURCES - subtype of market 
    "val":60 
    "base":"PERCENT_TO_ALL"   //growth bonus for castle 
  } 
}

Reasons:

1. Looks too similar to bonus system, despite not being such
2. Problems with more complex buildings that can’t be described with several numbers

[Macron1]

What I’m OK with:

"building" : {
    "cost" : { ... },
    "produces" : { ... },
    "givesSpells" :  5, 4, 3, 2, 1 ]
     ...
}

Like “givesPrimSkill”: [2,0,0,0]? // + 2 attack and 0 to others

[Ivan]

Something like that. Specifically about permanent bonuses to heroes - I placed them into “hero visitables” category and planning to implement them as part of configurable objects (whenever I get to finishing it).

See wiki for details: wiki.vcmi.eu/index.php?title=Building_bonuses

If you want to work on this - I’m OK with everything on that page but trade buildings. Let’s start with something simple and see how that will turn out.

On trade buildings: their functionality is too complex. I would like to have description of artifact trader to sound like “3 random artifacts of treasure class, 3 random minors and 1 random major” but I’m not sure how to describe this in a generic form via json and to implement this in code.
So for now I’d rather keep them hardcoded (possibly - selectable as “unique buildings” from that page)

[Macron1]

Something like that. Specifically about permanent bonuses to heroes - I placed them into “hero visitables” category and planning to implement them as part of configurable objects (whenever I get to finishing it).

See wiki for details: wiki.vcmi.eu/index.php?title=Building_bonuses

If you want to work on this - I’m OK with everything on that page but trade buildings. Let’s start with something simple and see how that will turn out.

On trade buildings: their functionality is too complex. I would like to have description of artifact trader to sound like “3 random artifacts of treasure class, 3 random minors and 1 random major” but I’m not sure how to describe this in a generic form via json and to implement this in code.
So for now I’d rather keep them hardcoded (possibly - selectable as “unique buildings” from that page)

I will wait some time to start with buildings, it’s too complex to handle it without preparations.

I pulled commits for boats support for faction in GITHUB.
This is untested feature, so if someone wants to compile my fork and test new boats support it’s OK.
I will wait until boats support is corrected and checked. And only then will move to something big and new.

Ivan, can you look for my schema for faction.json on Github? It doesn’t work as it is now. What I done wrong?

PPS The new boats on test new faction and standard factions seem to work. Town builds right type of ships with new appearance. So HOTA pirate ship can also be realized for Cove, if feature is finished.

[AVS]

I pulled commits for boats support for faction in GITHUB.

1. it will be good that U will follow guidelines even in your repo. Please keep your delelop head equal upstream and implement features in separate branches based on develop head.
2. If changes are small, then after synch develop head with upstream push to your “develop” and make pull request.
   (Currently your changes are based on your previous work - that is not in synch with upstream despite the fact that it have been merged - and on upstream head)

[Macron1]

I pulled commits for boats support for faction in GITHUB.

1. it will be good that U will follow guidelines even in your repo. Please keep your delelop head equal upstream and implement features in separate branches based on develop head.
2. If changes are small, then after synch develop head with upstream push to your “develop” and make pull request.
   (Currently your changes are based on your previous work - that is not in synch with upstream despite the fact that it have been merged - and on upstream head)

Sorry, i’m not so good in these terms.
Before work I made something on github, that I think made my repository equal to current “develop”, I think (I don’t remember how it’s called). Then I again cloned it to my desktop to new folder. So it may contain last changes in “develop” I think.
(2) - I made several commits with files divided by their location. This is not what I must have done?
(you mean that my rep is not in sync with upstream?) What is “upstream head”?

[Ivan]

You need to have two defined “remotes”:

1. “origin” which points to github.com/Macron1Robot/vcmi.git
2. “upstream” pointing to github.com/vcmi/vcmi.git
   (names may be different)

To start a new feature you need to do this (look your GUI for options with such name)

• pull from upstream
• clone upstream/develop into
• switch to

When you want to upload your code to github use:

• push to origin

[Macron1]

You need to have two defined “remotes”:

1. “origin” which points to github.com/Macron1Robot/vcmi.git
2. “upstream” pointing to github.com/vcmi/vcmi.git
   (names may be different)

To start a new feature you need to do this (look your GUI for options with such name)

• pull from upstream
• clone upstream/develop into
• switch to

When you want to upload your code to github use:

• push to origin

I think I made these things (pull from upstream - through github),
cloned develop into
Only not created upstream.

[AVS]

I think I made these things (pull from upstream - through github),
cloned develop into
Only not created upstream.

Push the result. There is nothing changed yet in you cloned repo.