Forum index VCMI Project - Heroes 3: WoG recreated
Forum of the project aiming to recreate best turn-based strategy ever!

FAQFAQ  SearchSearch  MemberlistMemberlist  UsergroupsUsergroups  StatisticsStatistics
RegisterRegister  Log inLog in  AlbumAlbum  DownloadDownload

Previous topic :: Next topic
Configurable Secondary Skills
Author Message
King_Crusher

Joined: 17 Mar 2013
Posts: 26
Posted: 2016-02-10, 23:23   Configurable Secondary Skills

I want to implement make secondary skills moddable, by introducing JSON config for them.

While looking at the code, I wondered something. In CGHeroInstance::updateSkill(), two different ways of getting a bonus object are used:
Code:
// used by luck, morale and diplomacy
Bonus *b = getBonusLocalFirst(Selector::type(Bonus::SURRENDER_DISCOUNT).And(Selector::sourceType(Bonus::SECONDARY_SKILL)))

// used by all others
Bonus * b = getExportedBonusList().getFirst(Selector::typeSubtype(Bonus::SECONDARY_SKILL_PREMY, which).And(Selector::sourceType(Bonus::SECONDARY_SKILL)))

Why is this the case, and does it have to be that way?

Also, I'm thinking about the JSON format for secondary skills. At the moment I have something like this in mind:
Code:
{
"name":"skill name",

"basicIcon":"icon name",
"advancedIcon":"icon name",
"expertIcon":"icon name",

"basicBonuses":[<bonus format>,...],
"advancedBonuses":[<bonus format>,...],
"expertBonuses":[<bonus format>,...],

"whitelistFactions": ["faction",..], // only necropolis heroes can have Necromancy
"blacklistFactions": ["faction",..], // necropolis heroes can't have Leadership

"maxlevelsMight":0, // for wisdom and spell schools
"maxlevelsMagic":0
}

What do you think of this?
 
     
SXX 

Age: 25
Joined: 04 Jul 2014
Posts: 290
Posted: 2016-02-11, 06:46   

Levels must be present as array even if we don't support 3+ level skills yet:
Code:
        "levels" : {
            "basic":{
                "bonuses":[<bonus format>,...],
                "icon":"icon name",
                etc...
            },
            "advanced":{
                "bonuses":[<bonus format>,...],
                "icon":"icon name",
                etc...
            }
        },
 
     
SXX 

Age: 25
Joined: 04 Jul 2014
Posts: 290
Posted: 2016-02-11, 07:00   

About bonuses. Code is inconsistent there because this way (with help of @AVS) I attempted to fix corruptions within bonus system. Issue was that some of that code modified "cache" of bonus lists instead of actual data. You can find related commits with "git blame". Sadly looking on that code now I'm no longer sure there is no code with wrong behavior. :oops:

Unfortunately it's impossible to improve bonus system further until noncopyableBonusSystemNode branch merged. And that won't happen till we start breakage of save format. :-|

PS: So if you decide to change something related to bonuses please make sure to work on top of that branch.
 
     
AVS 

Joined: 25 Feb 2011
Posts: 543
Location: Russia
Posted: 2016-02-11, 08:21   Re: Configurable Secondary Skills

King_Crusher wrote:
I want to implement make secondary skills moddable, by introducing JSON config for them.

While looking at the code, I wondered something. In CGHeroInstance::updateSkill(), two different ways of getting a bonus object are used:
Code:
// used by luck, morale and diplomacy
Bonus *b = getBonusLocalFirst(Selector::type(Bonus::SURRENDER_DISCOUNT).And(Selector::sourceType(Bonus::SECONDARY_SKILL)))

// used by all others
Bonus * b = getExportedBonusList().getFirst(Selector::typeSubtype(Bonus::SECONDARY_SKILL_PREMY, which).And(Selector::sourceType(Bonus::SECONDARY_SKILL)))

Why is this the case, and does it have to be that way?

Second is right way to modify (only modify existing objects) local bonuses. (getExportedBonusList return actual container) Be careful with client-server synchronization.

King_Crusher wrote:

Also, I'm thinking about the JSON format for secondary skills. At the moment I have something like this in mind:
Code:
{
"name":"skill name",

"basicIcon":"icon name",
"advancedIcon":"icon name",
"expertIcon":"icon name",

"basicBonuses":[<bonus format>,...],
"advancedBonuses":[<bonus format>,...],
"expertBonuses":[<bonus format>,...],

"whitelistFactions": ["faction",..], // only necropolis heroes can have Necromancy
"blacklistFactions": ["faction",..], // necropolis heroes can't have Leadership

"maxlevelsMight":0, // for wisdom and spell schools
"maxlevelsMagic":0
}

What do you think of this?


I like different design
Code:

{
"base":{<attributes for all skill levels if any>},
"basic":{"icon":"", "bonuses":{} },
"advanced": ...
}


Also try to avoid json arrays - they are bad for modding.
 
     
Macron1 

Joined: 02 Apr 2013
Posts: 574
Posted: 2016-02-11, 09:02   Re: Configurable Secondary Skills

King_Crusher wrote:
I want to implement make secondary skills moddable, by introducing JSON config for them.


Great idea, i hope you can make it!

King_Crusher wrote:


"whitelistFactions": ["faction",..], // only necropolis heroes can have Necromancy
"blacklistFactions": ["faction",..], // necropolis heroes can't have Leadership

That's bad idea, since secondary skills mod doesn't know how many new factions there will be.
Now secondary skills are written in heroClass:
http://wiki.vcmi.eu/index..._Classes_Format
Code:

    // Chance to get specific secondary skill on level-up
    // Skills not listed here will be considered as unavailable, including universities

I think it's wrong idea, i suppose you change this to that way:
Code:

// Chance to get specific secondary skill on level-up
// Skills with chance to get =0 here will be considered as unavailable, including universities
// Skills non listed here will have default chance on level-up
_________________
I'm not a member of VCMI developer group and my posts are not official. I'm just a fan.
 
     
Macron1 

Joined: 02 Apr 2013
Posts: 574
Posted: 2016-02-11, 09:17   Re: Configurable Secondary Skills

King_Crusher wrote:

"maxlevelsMight":0, // for wisdom and spell schools
"maxlevelsMagic":0

?
I think it's not needed. Required secSkills/primarySkills for skill can be written in levels.

Secondary skill needs "affinity" : "magic"/"might" altrough.
_________________
I'm not a member of VCMI developer group and my posts are not official. I'm just a fan.
 
     
King_Crusher

Joined: 17 Mar 2013
Posts: 26
Posted: 2016-02-12, 15:16   

AVS wrote:
I like different design
Code:

{
"base":{<attributes for all skill levels if any>},
"basic":{"icon":"", "bonuses":{} },
"advanced": ...
}


Also try to avoid json arrays - they are bad for modding.
Will do it that way instead.

Macron1 wrote:

That's bad idea, since secondary skills mod doesn't know how many new factions there will be.
Now secondary skills are written in heroClass:
http://wiki.vcmi.eu/index..._Classes_Format
Code:

// Chance to get specific secondary skill on level-up
// Skills not listed here will be considered as unavailable, including universities

I think it's wrong idea, i suppose you change this to that way:
Code:

// Chance to get specific secondary skill on level-up
// Skills with chance to get =0 here will be considered as unavailable, including universities
// Skills non listed here will have default chance on level-up
It is true that secondary skills won't know all factions. However, hero classes won't know all secondary skills either. I think the best solution is doing it as you suggested; secondary skills need to be explicitly banned from the hero class. However, it would be troublesome if someone makes a mod with a secondary skill only meant for one faction (like necromancy), because the modder would then have to mod all existing hero classes (even other ones added by mods). A field in the secondary skill config, called "defaultChance" would solve that issue. That value will be used, if it is not expicitly specified in config.

Macron1 wrote:
?
I think it's not needed. Required secSkills/primarySkills for skill can be written in levels.

Secondary skill needs "affinity" : "magic"/"might" altrough.
Wisdom and spell schools have the property that heroes are guaranteed to get them as a choice every n:th level; that's what I intended these fields for. What would the "affinity" field be useful for?
 
     
Macron1 

Joined: 02 Apr 2013
Posts: 574
Posted: 2016-02-12, 19:45   

King_Crusher wrote:
What would the "affinity" field be useful for?

Maybe it not needed, I don't remember now.
In "heroClass" we have "affinity", which determines, might or magic skills hero will preferably exercise.
_________________
I'm not a member of VCMI developer group and my posts are not official. I'm just a fan.
 
     
King_Crusher

Joined: 17 Mar 2013
Posts: 26
Posted: 2016-02-13, 00:17   

A question:

How do translations work? The same as other mods (by overriding JSON config) or somehow else? I couldn't figure it out by looking through the code, and I need to know this to implement changes to secondary skills...
 
     
AVS 

Joined: 25 Feb 2011
Posts: 543
Location: Russia
Posted: 2016-02-13, 06:48   

King_Crusher wrote:
A question:

How do translations work? The same as other mods (by overriding JSON config) or somehow else?


By overriding JSON. Also its recommended to have translatable strings in separate JSON files (but in same schema) - f.e. see "bonuses.json" "bonuses_texts.json" .
 
     
King_Crusher

Joined: 17 Mar 2013
Posts: 26
Posted: 2016-02-23, 20:45   

At the moment I have gotten stuck on handling heroes levelling up and proposing new skills. It is really difficult because there are a lot of special cases to handle, and even more so due to the fact that it should be configurable whether or not those special cases apply...

I will likely refactor the skill on level up selection away from CGHeroInstance, because I think it should be possible to override the default behaviour when scripting is supported.
 
     
Macron1 

Joined: 02 Apr 2013
Posts: 574
Posted: 2016-03-07, 07:32   

How is the progress going?
Will it be made with same bonuses system as spells and hero/creature bonuses?
_________________
I'm not a member of VCMI developer group and my posts are not official. I'm just a fan.
 
     
King_Crusher

Joined: 17 Mar 2013
Posts: 26
Posted: 2016-03-07, 19:03   

Still stuck at the same problem I mentioned above. And yes, it will use the same bonus system.
 
     
Macron1 

Joined: 02 Apr 2013
Posts: 574
Posted: 2016-03-07, 19:56   

King_Crusher wrote:
Still stuck at the same problem I mentioned above. And yes, it will use the same bonus system.


Some of standard skills allow SECONDARY_SKILL_PREMY bonuses in %, and some are not. Maybe firstly make them all resposible to bonuses % and move level % to json configs.
Maybe standard secondary skills could be left as they are now, and only add new skills modding posibility?
_________________
I'm not a member of VCMI developer group and my posts are not official. I'm just a fan.
 
     
Macron1 

Joined: 02 Apr 2013
Posts: 574
Posted: 2016-06-28, 08:38   

Are there any news in implementing secondary skills modding and adding?
Don't see GitHub commits on this topic :-?
_________________
I'm not a member of VCMI developer group and my posts are not official. I'm just a fan.
 
     
Display posts from previous:   
Reply to topic
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You can download files in this forum
Add this topic to your bookmarks
Printable version

Jump to:  

Powered by phpBB modified by Przemo © 2003 phpBB Group
Template Chronicles modified by Nasedo modified by Tow.
© VCMI Team
Page generated in 0.07 second. SQL queries: 12