search

Thalassicus / cep-bnw

Civ V Communitas Expansion Pack
32 stars 22 forks source link

City State Diplomacy Mod Compatibility #157

Open stackpoint opened 10 years ago

stackpoint commented 10 years ago

(Reminder: Set USING_CSD in Cat_Options.sql to 1)

I want to try to get these mods compatible with each other and since CSD is mostly data edits (v14) that means most of the problems are on our end.

I'll head any development on this front.

See: http://forums.civfanatics.com/showthread.php?t=516558

skodkim commented 10 years ago

Also working on it. Two issues currently:

1: Making gold gifts unavailable by activating the CSD option in CEP's option file again, see http://forums.civfanatics.com/showthread.php?t=519695

2: Merging the cityview.lua file from the two mods, see http://forums.civfanatics.com/showpost.php?p=13025639&postcount=1427

\Skodkim

skodkim commented 10 years ago

Actually there's one more thing. A lot of incompaibility problems (reports on missing CSD units) may also be due mod load order.

\Skodkim

Edit: Sorry, made a mistake in the test.

stackpoint commented 10 years ago
  1. Should be easy, just need to create an SQL file in the diplomacy folder. I'll start on it tomorrow.
  2. Lua should be trickier but I can make a CSD check in our CityView.lua file. I'll go check what the differences are.
  3. I'll add a reference in CAT.
stackpoint commented 10 years ago

Hm, it seems like there were a ton of lua files added in CSD v15...

stackpoint commented 10 years ago

v14 should be 92.4% compatible with that commit. Just set the CSD option in the CAT_Options.sql file and it should work. The SQL is set to ignore CEP's gold gifting values so that you can use CSD options file to set the balance.

GrantSP commented 10 years ago

Is it with or without the DLL?

stackpoint commented 10 years ago

Both should work on v14. I tested with the DLL though.

skodkim commented 10 years ago

Fantastic news. I also see that you've made a reference to CSD in CAT. That was also my conclusion after a few tests: If CAT runs after CSD you don't see the missing CSD units bug that many users have reported. You do if its the other way around.

Have you seen Gazebos post on searching through his cityview.lua in v15 to find changes he made: http://forums.civfanatics.com/showpost.php?p=13025697&postcount=1428

If you search for "servant" or "diplomat" you'll find that he only added three Blocks of code and it seems easy enough to merge. If you could do a CSD check in the lua file then...

I don't think the other lua files are relevant but I haven't tested. None of them are found in CEP.

\Skodkim

stackpoint commented 10 years ago

It should be compatible with v15 now. However, I haven't had a chance to do any extensive testing with the changes:

See: https://github.com/Thalassicus/cep-bnw/commit/464bc957c7e9b88df4d09c629fb2f1239157cf44

skodkim commented 10 years ago

Great. Maybe I'll have the chance to test later.

\Skodkim

stackpointer notifications@github.com skrev:

It should be compatible with v15 now. However, I haven't had a chance to do any extensive testing with the changes:

See: https://github.com/Thalassicus/cep-bnw/commit/464bc957c7e9b88df4d09c629fb2f1239157cf44

Reply to this email directly or view it on GitHub: https://github.com/Thalassicus/cep-bnw/issues/157#issuecomment-33310706

Sendt fra min Android telefon med K-9 Mail. Undskyld hvis jeg er lidt kortfattet.

stackpoint commented 10 years ago

Nevermind, I'm having trouble getting it working. Probably from one of my non CSD changes to the file.

stackpoint commented 10 years ago

Nevermind, it was a minor syntax error. It should work now.

stackpoint commented 10 years ago

Okay, did some more minor testing. Looks like you might(?) not need to delete the CityView.lua file in CSD v15 since CAT loads after CSD. But it all seems to work fine with both v14 and v15.

GrantSP commented 10 years ago

The new code looks to be working fine. I included the v 15 in my mod includes and tried a game and for the first time ever I had a working CEP/CSD game.

Well done guys I think this is a brilliant addition to the game.

skodkim commented 10 years ago

It seems there's a small display issue: http://forums.civfanatics.com/showpost.php?p=13028699&postcount=1461

\Skodkim

stackpoint commented 10 years ago

@GrantSP mentioned this to me through chat before. I'll take a closer look.

stackpoint commented 10 years ago

Okay, so the problem seems to be that CEP needs to set SPECIALIST_CIVIL_SERVANT to a specific IconString. The question is, which one? I don't believe CSD has a specific icon graphic for the civil servant so we can use any of the ones from here: http://forums.civfanatics.com/showpost.php?p=12632026&postcount=3

GrantSP commented 10 years ago

I really should start my day looking here first before going off and searching for clues! Its like the elves have come in the middle of the night and mended all the shoes! ;) I was thinking of the same thing, perhaps the files Cep_Icons.xml & Cat_Data.xml could have an addition to include the Civil Servant? The actual icon shouldn't matter too much, maybe reuse the Great Merchant one?

I'm just about to try something now. If you have already hit upon something let me now before I stuff something up. :)

GrantSP commented 10 years ago

Yep, got it. Just a small defining line in Cat_Data.xml

All I did was reuse the Great Merchant icon. All that is one of those heads in profile with a background colour, we could use any of them or if we want to be fancy make a new one with a different colour and use that.

stackpoint commented 10 years ago

I'll leave the commit to you then.

skodkim commented 10 years ago

I've got a bug that comes using CEP with whowards modified dll+CSD (v15.6). It only happens when I activate CEP though so it seems like its a problem on our side.

The bug I'm seeing is that I'm able to work tiles four tiles away in all cities from turn 1 (tested using IGE to expand city radius - also seen in normal game).

In an ongoing discussion with Whoward he mentioned that it is propably due to cityview.lua: http://forums.civfanatics.com/showpost.php?p=13034263&postcount=555

Can anyone confirm this behaviour?

\Skodkim

stackpoint commented 10 years ago

I can't sleep so I'm looking at this.

I took at Whoward's City Working Distance mod and it seems from his comments inside the xml that it has to do with MAXIMUM_BUY_PLOT_DISTANCE which CEP changes to 4.

    <Defines>
        <Update>
            <!-- Buy/Work tile maximum distance from city centre (default is 3) -->
            <!-- Min value is 2, max value is 5 (these are hard-coded in the DLL) -->
            <Where Name="MAXIMUM_BUY_PLOT_DISTANCE"/>
            <Set Value="3"/>
        </Update>
    </Defines>

You can test this by modifying the value within CEP. Something interesting is that I can't buy tiles past the third ring with just CEP active. That might definitely has something to do with our CityView.lua file. Just looked at the GEM files. More changes that need to be activated.

skodkim commented 10 years ago

Not in front of my game computer right now but I actually proposed that MAXIMUM_BUY_PLOT_DISTANCE could be the culprit. Will test tonight if possible.

\Skodkim

stackpoint commented 10 years ago

I was also reflecting on what you mentioned on the forums about the policies.

While it might not be practical to disable all the CEP patronage policies in lieu of CSD's, it shouldn't be difficult to modify our POLICY_PHILANTHROPY (regarding gold gifts) depending on whether USING_CSD Is active. However, the difficult question is what would we change it to? I took a look at CSD's modified policies and they actually are pretty similar to ours in many respects and I couldn't figure out a different philanthropy policy from them.

skodkim commented 10 years ago

I don't think we should change the policy (or any of the others) if USING_CSD is active. My idea was that we don't make any changes to the policies CSD change if USING_CSD is active - that way we let CSD control it. The same could actually be said about wonders (I only think CSD changes the mausoleum). I'm not familiar with what we do to Sweden's trait but I think CSD has an option for that as well.

Does that sound right?

\Skodkim

stackpoint commented 10 years ago

I understand what you're saying but the problem with not changing the policies is that the policy changes are scattered throughout the mod and it would be difficult to add exceptions to all of them. At the very least you'd have to rewrite the patronage xml data edits using SQL for the checks and it would just be a lot simpler to change just that one policy.

skodkim commented 10 years ago

Agreed. If we could just mod POLICY_PHILANTHROPY we'd have solved the most obvious issue.

\Skodkim

skodkim commented 10 years ago

Edit: What Am I talking about? It works and I'm going to make a commit :smile:

Hi I was trying to make BUILDING_HAGIA_SOPHIA and BUILDING_JADE_HALL compatible with CEP by adding yields to 'SPECIALIST_CIVIL_SERVANT' with the two buildings if USING_CSD=1.

I added the two sql statements below. I am, however, a total newb at sql and soehow I missed something as the sql adds yields no matter what USING_CSD is. Could someone help:

Thanks in advance.

\Skodkim

Added to CEC__Start.sql: --Compatibility with CSD for BUILDING_HAGIA_SOPHIA INSERT INTO Building_SpecialistYieldChanges (BuildingType, SpecialistType, YieldType, Yield) SELECT 'BUILDING_HAGIA_SOPHIA', 'SPECIALIST_CIVIL_SERVANT', 'YIELD_FAITH', 1 WHERE EXISTS (SELECT Value FROM Cep WHERE Type='USING_CSD' AND Value= 1 );

Added to CEL_End.sql: --Compatibility with CSD for BUILDING_JADE_HALL INSERT INTO Building_SpecialistYieldChanges (BuildingType, SpecialistType, YieldType, Yield) SELECT 'BUILDING_JADE_HALL', 'SPECIALIST_CIVIL_SERVANT', 'YIELD_SCIENCE', 1 WHERE EXISTS (SELECT Value FROM Cep WHERE Type='USING_CSD' AND Value= 1 ) ;

stackpoint commented 10 years ago

Looks like you got it. To put code into code blocks use: https://help.github.com/articles/github-flavored-markdown#syntax-highlighting

skodkim commented 10 years ago

Philantropy policy updated:

skodkim commented 10 years ago

I think we should close this issue. With the upcoming changes we seem to have accomplished everything you could ask for :smile:

\Skodkim

skodkim commented 10 years ago

@stackpoint I noticed that you (?) created a new line in Cat_Options.sql setting USING_CSD=1 if the CSD version without dll is used and USING_CSD=2 if the dll version is used.

I was doing a text update to CSD.sql and was wondering if this wouldn't be a problem with the compatibility. Right now there's checks for USING_CSD=1 in the code (e.g. CEP_End.sql) and as far as I can tell this means they won't trigger if the dll version is used.

If so a solution could be changing the second line in Cat_Options.sql to: UPDATE Cep SET Value = 1 WHERE Type='USING_CSD_DLL' AND EXISTS(SELECT Type FROM Specialists WHERE Type='SPECIALIST_CIVIL_SERVANT');

Then we could check for USING_CSD_DLL in the places in the code where it makes a difference if the dll version is used and check for USING_CSD in the places where it just matters whether you use CSD (e.g. CEP_End.sql)

\Skodkim

stackpoint commented 10 years ago

You're right, I had forgotten to add checks for the new state I created in the SQL. I had only added them to the LUA files. Right now there are three checks that matter:

USING_CSD = 0  CSD is not loaded
USING_CSD > 0  CSD is loaded
USING_CSD = 2  CSD with DLL is loaded (for specialist specific changes)

See my fix here: https://github.com/Thalassicus/cep-bnw/commit/6eee996d262a885bf8fa749ec9d8e82ad11f5b53

skodkim commented 10 years ago

Great. Made a small text fix for Philantropy as "School of Scribes" has been renamed "Chanceries" but only for the dll version: https://github.com/Thalassicus/cep-bnw/commit/e785446c47c45caac485731e77b40519502d5788

GrantSP commented 10 years ago

I don't believe the improvement from using a Great Diplomat on a strategic resource owned by a CS gets displayed anywhere. The TopPanel doesn't show any increases either in the bar itself or the tooltip that appears when mousing over the Resource icons.

EDIT: The notifications show the resources are being added to the empire, but there doesn't appear to be any display other than that.

GrantSP commented 10 years ago

There appears to be a TXT_KEY missing for the DLL version of CSD version 21.

Philanthropy just shows this in the Abilities section: TXT_KEY_POLICY_PHILANTHROPY_HELP_CSD_DLL +1 City Happiness from School of Scribes

Maybe that was to do with this change.

There is this TXT_KEY_POLICY_PHILANTHROPY_HELP_EXTRA in the CSD.sql which has a couple of lines of CSD specific text. That defines the 2nd line shown for this policy. Is this needed? Is this supposed to be removed now the new compatibility checks are in place?

I'm not certain what the text is actually supposed to say. If anyone can enlighten me I could easily add it in.

GrantSP commented 10 years ago

The new buildings this mod provides are geared towards Diplomacy and are using some new columns that are added to the Buildings table.

RaiseCityStateInfluenceToAlly RaiseCityStateInfluenceToFriend SpaceshipProductionMod SpaceshipPurchaseMod AttackBonusTurns BaseFreeUnits FaithToVotes CapitalsToVotes DoFToVotes RAToVotes NumInfPerEra GreatDiplomatRateModifier ImprovementLeagueVotes

It would good if we could write some sql to assign higher Diplomacy values to these buildings based on what their purpose is. As it is now most of them get the same values as for Offense which means they will considered by conquest oriented civs as much as diplomats.

Not sure if these sql commands would need to make a check for the new building effects first or risk crashing the file if CSD doesn't exist.

GrantSP commented 10 years ago

Possible new UI error with the latest (v22) CSD mod. The CityView.lua is formatting the TXT_KEY_CITYVIEW_BUILDING_SPECIALIST_YIELD in a weird way, similar to what was seen with EUI and the values displayed with that mod. cityview_specialist_text

I know the way that particular lua code is written is slightly different, but I was under the impression it was just another way of doing a similar thing. Maybe not. Are the yields being formatted correctly here? Is it a case of load order, CEP before CSD or the other way around?

skodkim commented 10 years ago

That looks weird and I have no idea why it looks like that.

CSD runs before CEP.

skodkim commented 10 years ago

How do you know this was due to CSD v22?

GrantSP commented 10 years ago

Well I'm not 100% sure, that's why I said: "Possible new UI error with the latest (v22) CSD mod."

My methods of testing new mods are pretty simple though, the only mods running were ours and CSD. I'm going to spend today looking into it, it could quite possibly be an anomaly that I can't replicate.

GrantSP commented 10 years ago

It must have been some anomaly, maybe from an uncleared cache? I don't know. It can't be replicated at the moment. Disregard it completely until it perhaps shows again.

GrantSP commented 10 years ago

CSD has now added in a new 'Settler' mod which provides better settlers based on techs reached. Pioneers and Colonists now give 3 or 5 population and 1st tier buildings on founding cities and this implementation means the Exploration policies which provide similar options are redundant when using CSD.

Any ideas as to what to change these policies into when the player also has CSD?