Is there any guide for VP development?

[Vaderkos]

Hi, I'm Software Developer, and after playing with VP numerous time I want to contribute.
I'm unsure what can be touched in VP repo.

1. Are there any guidelines?
2. Code Style Reference.
3. Crucial Parts and what should or shouldn't be touched?
4. Code structure Reference.

Any honorable mentionions and pitfalls>?

 

[ilteroi]

there are some hints in the readme. other than that, not much. no style guide either, it's legacy hell. and there are zero testcases because it's hard to run the gamecore in isolation. also keep in mind this is a hobby project ...

now that you mention it, i might write an intro on code structure, but honestly the biggest issue is lack of automated testing.

 

[Vaderkos]

As I'm looking down the code, there is a huge amount of boilerplate from old era. Like xmls, huge files, lua scripts.

 

[Vaderkos]

there are some hints in the readme. other than that, not much. no style guide either, it's legacy hell. and there are zero testcases because it's hard to run the gamecore in isolation. also keep in mind this is a hobby project ...

now that you mention it, i might write an intro on code structure, but honestly the biggest issue is lack of automated testing.

I'm also wondering what files can be changed (I mean licensing)

 

[axatin]

As I'm looking down the code, there is a huge amount of boilerplate from old era. Like xmls, huge files, lua scripts.

xml and lua are used to allow modding parts of the game without changing the source code. You can check out the "core concepts" section of this modder's guide to get an overview over the structure.

 

[Agiwan]

@seroperson has written some guidelines specifically for the multiplayer part and reverse-engineering if you're interested: Link

 

[azum4roll]

Check the stickied "Modding Guide for VP 4.0+" for file structure and basic coding style of specifically the (2) Vox Populi mod.

There's no plan to clean up the others for now.

As for the DLL, I don't see any specific coding style other than
1) use tabs
2) open curly bracket on new line
3) space between variable and operator

Function names are in both lower and upper case and are in different order in the .h and .cpp files. It already looked like this in vanilla and nobody bothered to clean it up.

 

[Vaderkos]

Check the stickied "Modding Guide for VP 4.0+" for file structure and basic coding style of specifically the (2) Vox Populi mod.

There's no plan to clean up the others for now.

As for the DLL, I don't see any specific coding style other than
1) use tabs
2) open curly bracket on new line
3) space between variable and operator

Function names are in both lower and upper case and are in different order in the .h and .cpp files. It already looked like this in vanilla and nobody bothered to clean it up.

Thx for the info.

What is the case with .sql and .xml files for database changes. Why formats are mixed? Is there any case or pitfall to go with one or another. (Looks like xml come from mod editor?)

Also I've encountered numerous number values within project, and not many have comments that describe what such values are...

Is there any intellisense within project?

Which sqllite version syntax is supported?

 

[axatin]

What is the case with .sql and .xml files for database changes. Why formats are mixed?

See the explanation in the thread mentioned by azum4roll

Also I've encountered numerous number values within project, and not many have comments that describe what such values are

For some of the values there are comments in (1) Community Patch\Core Files\Core Values\CoreDefines.sql.

Some general considerations: Having the possibility to make changes to the mod by editing the database is an important principle, so do not hard-code numbers or abilities in c++. Constant values should always be in the "Defines" table, and all the abilities of buildings/civs/policies etc. must also be in the database (check for instance the various tables starting with "Building" to get a feeling for it).

Community Patch and Vox Populi use the same c++ code and the same database structure (but different database content) and when making changes to VP it's important to make sure the changes do not affect Community Patch (except for AI improvements). When it's necessary to implement a different logic for Community Patch and VP in the code, use

if (MOD_BALANCE_VP)

 

[azum4roll]

sqlite version is 3.7.11 (yes it's ancient)
lua version is 5.1.4

 

[Vaderkos]

I'm thinking about simple things right now.

Can I make PR with:

1. Some sql simplifications
2. As I look the code I don't see any indexes on tables. So I'd add creating indexes in Community Patch on mostly used table columns basing on size and what I see in code?

-- Get existing indexes (not autoindexes)
SELECT COUNT(*) FROM sqlite_master WHERE type = 'index' and name NOT LIKE '%autoindex%';

-- Get table names order by pgsize
SELECT name, sum(pgsize) as size from dbstat GROUP BY name ORDER BY size DESC;

 

[N.Core]

What is the case with .sql and .xml files for database changes. Why formats are mixed? Is there any case or pitfall to go with one or another. (Looks like xml come from mod editor?)

The mod editor supports both XML and SQL. Many people still prefer XML as it's easier to read and also used anywhere outside Civ V modding so people are already familiar with it. Also, the game loads XML files faster than SQL in some cases. However, the downside of using XML is that it isn't as flexible to insert/update something into the database as SQL can do.

You can do many insertions/updates/deletes in bulk by just using SQL, but you have to do it one by one with XML.
The mod uses XML for things that are rarely to be changed, while SQL is for things that could be changed often as the mod keeps being updated.

I, for one, prefer to do all of the database modding in SQL rather than XML.

 

[N.Core]

1. Some sql simplifications
2. As I look the code I don't see any indexes on tables. So I'd add creating indexes in Community Patch on mostly used table columns basing on size and what I see in code?

1. As for this, the mod has just finished merging. So there are some fresh codes without any commit history on it. I'm not sure what needs to be more simplified, but I suggest consulting with some of the active modders here (especially @azum4roll) about the guidelines of the mod files' structure.
2. What's this for? Are there any benefits to doing so?

 

[Vaderkos]

1. As for this, the mod has just finished merging. So there are some fresh codes without any commit history on it. I'm not sure what needs to be more simplified, but I suggest consulting with some of the active modders here (especially @azum4roll) about the guidelines of the mod files' structure.
2. What's this for? Are there any benefits to doing so?

Yeah my first PR just got merged . https://github.com/LoneGazebo/Community-Patch-DLL/pull/10595/files
2. Yes, when using indexes database changes the way it stores data, it speeds up quering data from db if columns from index are used.
(For example from real world imagine library: You want to find book and you know it's author so you traverse all the shelfs till you find it. Index is like a card that you create before that says which author corresponds to which shelf. So when someone want's to find a book by author, you can look into such card and eactly know which shelfs to check instead of going through all.)
From information on internet looks like sqlite uses some sort of B-Tree. https://www.sqlitetutorial.net/sqlite-index/
I've looked at autoindexes that db creates automatically and it looks like such optimization is not needed as it almost everywhere creates index on Type field.
But if you know some queries from code that checks some other column very often, please mention it and I can look if index is needed.

 

[N.Core]

Hmm... Maybe you can try it yourself to see if it has some benefits for the game because loading queries only happen during the loading of the mod (see Stopwatch.log to see if there are any improvements in loading times). If it saves only a fraction of time, I don't see any benefit in doing it other than making the DB file bigger.

Also I've encountered numerous number values within project, and not many have comments that describe what such values are...

Yeah, this is planned. I also wanted to do some thorough documentation work for the added features to the mod, to make it friendly to newcomer modders. Some features were implemented by old modders without any extra explanation on how they work or what they are for, requiring you to go to the cpp file itself.

Is there any intellisense within project?

No, sadly. It's helpful if someone can make one for Lua API stuff.

 

[azum4roll]

The tables in Civ5 (a couple of hundred rows at most) are not big enough to warrant the use of indices (except the text and Defines tables which probably already have them?)

Note that many columns and tables already exist in base Civ5 and those of course won't have any comments.

 

[Vaderkos]

See the explanation in the thread mentioned by azum4roll

For some of the values there are comments in (1) Community Patch\Core Files\Core Values\CoreDefines.sql.

Some general considerations: Having the possibility to make changes to the mod by editing the database is an important principle, so do not hard-code numbers or abilities in c++. Constant values should always be in the "Defines" table, and all the abilities of buildings/civs/policies etc. must also be in the database (check for instance the various tables starting with "Building" to get a feeling for it).

Community Patch and Vox Populi use the same c++ code and the same database structure (but different database content) and when making changes to VP it's important to make sure the changes do not affect Community Patch (except for AI improvements). When it's necessary to implement a different logic for Community Patch and VP in the code, use

if (MOD_BALANCE_VP)

You've mentioned MOD_BALANCE_VP in C++. Is this value exposed to lua somewhere. As I look down the Overrides in Community patch and Vox Populi lua in ActionInfoPanel.lua there is a little change that could be done just in CP if such value is exposed to keep only 1 Overrides file.

Something like

if MOD_BALANCE_VP then
    player:EndTurnsForReadyUnits(true)

    if UIManager:GetShift() then
        player:EndTurnsForReadyUnits(false)
    end
end

Spoiler :

 

[axatin]

That's a good idea, it would be great if the number of files could be reduced.

MOD_BALANCE_VP corresponds to an entry in the database (see screenshot) and you can access it in lua as follows:

GameInfo.CustomModOptions("Name = 'BALANCE_VP'")().Value

 

[Vaderkos]

That's a good idea, it would be great if the number of files could be reduced.

MOD_BALANCE_VP corresponds to an entry in the database (see screenshot) and you can access it in lua as follows:

GameInfo.CustomModOptions("Name = 'BALANCE_VP'")().Value

View attachment 685584

Looks like CustomModOptions returns function that has to be called, what nil checks I have to perform here and is it available in base game or community patch or just vox populi?
And the other question what about file reloading, can i retrieve value one time at the start of file and cache it or I have to do it each time function called?

 

[axatin]

The function is also available in community patch (the returned value is 0 then). The value never changes during a game, so loading it at the start of the file should be fine.