Éa III, Sword & Sorcery: Developer's Thread

[Pazyryk]

I added whoward69's building unhappiness above (which is fine) but I also added two new events:

GameEvents.GetModHappiness(iPlayer) in CvPlayer::DoUpdateHappiness
GameEvents.GetModUnhappiness(iPlayer) in CvPlayer::GetUnhappiness

They are accumulators that would allow easy Lua control of player happiness. Sounds fine, right? Problem is, GetModUnhappiness fires at least 26 times and sometimes up to well over 100 times per player per turn (even for a minors!). And this is in the first 10 turns of a game. Jeez! Who coded this?

I said a while back that Lua can be faster than C++ (the same way a Honda can be faster than a Ferrari). This is exactly what I was thinking about.

Well, I'm not going to fix the dll. But this is probably not a good place for a GameEvent. So I pulled both of these back out. Probably should do this with new persisted members in CvPlayers.


Edit:
@ls612, if you want a dll job, you could recode this for me with two new persisted CvPlayer members:

m_iHappinessFromMod
m_iUnhappinessFromMod

and new Set and Get C++/Lua methods for both. Need it in a day or two though for me to use it on the Lua side.

 

[whoward69]

and sometimes up to well over 100 times per player per turn (even for a minors!). And this is in the first 10 turns of a game. Jeez! Who coded this?

Only a hundred times? Try putting one in CanMoveInto(), that'll fire over 2000 times in turn 0 for a standard map, 8 players, 16 minors setup!

(Which is why the version of that event in my DLL checks an additional column in the Units table called SendMoveInto - or somesuch)

 

[ls612]

I added whoward69's building unhappiness above (which is fine) but I also added two new events:

GameEvents.GetModHappiness(iPlayer) in CvPlayer::DoUpdateHappiness
GameEvents.GetModUnhappiness(iPlayer) in CvPlayer::GetUnhappiness

They are accumulators that would allow easy Lua control of player happiness. Sounds fine, right? Problem is, GetModUnhappiness fires at least 26 times and sometimes up to well over 100 times per player per turn (even for a minors!). And this is in the first 10 turns of a game. Jeez! Who coded this?

I said a while back that Lua can be faster than C++ (the same way a Honda can be faster than a Ferrari). This is exactly what I was thinking about.

Well, I'm not going to fix the dll. But this is probably not a good place for a GameEvent. So I pulled both of these back out. Probably should do this with new persisted members in CvPlayers.


Edit:
@ls612, if you want a dll job, you could recode this for me with two new persisted CvPlayer members:

m_iHappinessFromMod
m_iUnhappinessFromMod

and new Set and Get C++/Lua methods for both. Need it in a day or two though for me to use it on the Lua side.

Why not just use one, and let negative values represent unhappiness? That is something different from civ 4 to civ 5 which drives me insane, in civ 4 we just represented unhappiness as negative happiness.

 

[Pazyryk]

Why not just use one, and let negative values represent unhappiness? That is something different from civ 4 to civ 5 which drives me insane, in civ 4 we just represented unhappiness as negative happiness.

That's fine for us. So you have m_iHappinessFromMod and a single set of Get/Set methods (C++ and Lua), with negative value allowed. But you will need to add effect separately:

If positive, in CvPlayer::DoUpdateHappiness.
If negative, in CvPlayer::GetUnhappiness.

I think that is safer than subtracting from happiness. The mod may be adding up to -40 from Armageddon plus other effects. I'm not sure what happens if you drive the total value in DoUpdateHappiness into a negative value.

Only a hundred times? Try putting one in CanMoveInto(), that'll fire over 2000 times in turn 0 for a standard map, 8 players, 16 minors setup!

I bet that between turn times could be < 1 sec if this was cleaned up. There is just no reason to check unhappiness 100 times or a move option 2000 times. I can understand why it may need to check it more than once.

 

[ls612]

That's fine for us. So you have m_iHappinessFromMod and a single set of Get/Set methods (C++ and Lua), with negative value allowed. But you will need to add effect separately:

If positive, in CvPlayer::DoUpdateHappiness.
If negative, in CvPlayer::GetUnhappiness.

I think that is safer than subtracting from happiness. The mod may be adding up to -40 from Armageddon plus other effects. I'm not sure what happens if you drive the total value in DoUpdateHappiness into a negative value.

OK, that makes sense. I'll add the new methods probably tomorrow.

 

[Pazyryk]

New Sword_and_Sorcery commit.

Some Armageddon stuff and I (finally) added UI for the health/unhealth system - look in city view under yields (thanks to Nutty for icons!).

 

[ls612]

OK, that makes sense. I'll add the new methods probably tomorrow.

This is done now, and I've sent you a pull request for it.

 

[Pazyryk]

Well this has me stumped. The Get and Set methods persist, but they don't affect total unhappiness.

> Players[0]:GetUnhappiness()
0
> Players[0]:GetUnhappinessFromMod()
0
> Players[0]:SetUnhappinessFromMod(20)
> Players[0]:GetUnhappinessFromMod()
20
> Players[0]:GetUnhappiness()
0

I've looked at this for an hour and I can't see the problem. I've checked 5 times and the Lua method GetUnhappiness uses the dll method below. And (as you know since you coded it) the Lua method GetUnhappinessFromMod corresponds to the dll GetOtherUnhappiness. So GetOtherUnhappiness is capable of returning 20 to Lua, but it isn't capable of changing iUnhappiness below. I'm stumped.

int CvPlayer::GetUnhappiness(CvCity* pAssumeCityAnnexed, CvCity* pAssumeCityPuppeted) const
{
	int iUnhappiness = 0;

	// City Count Unhappiness
	iUnhappiness += GetUnhappinessFromCityCount(pAssumeCityAnnexed, pAssumeCityPuppeted);

	// Occupied City Count Unhappiness
	iUnhappiness += GetUnhappinessFromCapturedCityCount(pAssumeCityAnnexed, pAssumeCityPuppeted);

	// City Population Unhappiness
	iUnhappiness += GetUnhappinessFromCityPopulation(pAssumeCityAnnexed, pAssumeCityPuppeted);

	// Occupied City Population Unhappiness
	iUnhappiness += GetUnhappinessFromOccupiedCities(pAssumeCityAnnexed, pAssumeCityPuppeted);

	// Unit Unhappiness (Builders)
	iUnhappiness += GetUnhappinessFromUnits();

	//ls612: Handle all other unhappiness
#ifdef EA_UNHAPPINESS
	iUnhappiness += GetOtherUnhappiness();
#endif

	iUnhappiness /= 100;

	iUnhappiness += GetCulture()->GetPublicOpinionUnhappiness();

	// AI gets reduced Unhappiness on higher levels
	if(!isHuman() && !IsAITeammateOfHuman())
	{
		iUnhappiness *= GC.getGame().getHandicapInfo().getAIUnhappinessPercent();
		iUnhappiness /= 100;
	}

	return iUnhappiness;
}

 

[Pazyryk]

@ls612

Recompiled and tried again:

> Players[0]:GetUnhappiness()
0
> Players[0]:GetUnhappinessFromMod()
0
> Players[0]:SetUnhappinessFromMod(10)
> Players[0]:GetUnhappinessFromMod(10)
10
> Players[0]:GetUnhappiness()
0

It's beyond me how this is possible. So I'm going to go on to other things for a while. Maybe you can try this out yourself and see if you can track down the problem.

 

[Pazyryk]

There's a "manual hotfix" for the Media Pack. Go in and delete the directory FontIcons. It has two files in it but you want those gone.

These files are included in the main mod now because I change them often (as of the last 2 commits). If you don't do this you may or may not have the new health/unhealth icons (load order into the VFS is unpredictable).

 

[ls612]

@ls612

Recompiled and tried again:

> Players[0]:GetUnhappiness()
0
> Players[0]:GetUnhappinessFromMod()
0
> Players[0]:SetUnhappinessFromMod(10)
> Players[0]:GetUnhappinessFromMod(10)
10
> Players[0]:GetUnhappiness()
0

It's beyond me how this is possible. So I'm going to go on to other things for a while. Maybe you can try this out yourself and see if you can track down the problem.

Strange. It works for happiness though, right?

Edit: Wait, why is the value in getUnhappiness being divided by 100? The values returned by the sub methods don't seem to be returning unhappinessTimes100, they are returning raw unhappiness. I just checked the commit and I didn't add that statement.

 

[Pazyryk]

That's it. They are all "times 100" above the /100 (you can't tell from names). I just looked at TopPanel.lua and that is how they are used. I'll slip it below and commit if it tests out.

Edit: fixed, tested and committed.

 

[ls612]

That's it. They are all "times 100" above the /100 (you can't tell from names). I just looked at TopPanel.lua and that is how they are used. I'll slip it below and commit if it tests out.

Edit: fixed, tested and committed.

Gah, I'm halfway tempted to use Visual Studio's refactor/rename tools to fix all of the other happiness methods to end with times100.

 

[Pazyryk]

OK, I'm pretty much done. The last batch of spells went in with the last commit, all tested and functioning (at least in staged situations). The last thing I'm going to do is add help texts for all these. And then release it.

I know there are still TXT_KEYs littered around. Many of the tech helps are pointless because I moved all the info into automatically generated bullet points.

There are still holes in it, and heaven only knows about balance. What I feel good about is general stability. I think it's ready to hand off to testers.

 

[ls612]

OK, I'm pretty much done. The last batch of spells went in with the last commit, all tested and functioning (at least in staged situations). The last thing I'm going to do is add help texts for all these. And then release it.

I know there are still TXT_KEYs littered around. Many of the tech helps are pointless because I moved all the info into automatically generated bullet points.

There are still holes in it, and heaven only knows about balance. What I feel good about is general stability. I think it's ready to hand off to testers.

We should try and get volunteers to write the tech helps for us after release.

 

[Pazyryk]

Oops! Forgot to commit when I released!

The v1 release is committed now. Besides final texts, I removed the pdb file. That just means that you will have to set VFS=true when you add it back.

Also, the Media Pack (v4) up now is just slightly different then the v4 you already have. You can manually edit the "old" v4 by deleting the FontIcons folder with its two files. Then go into the .modinfo file and manually delete the two lines that reference those files. (I said the first part in post #190 above, but forgot to tell you the 2nd part.)

 

[Pazyryk]

I'm working on BrokenSky's problem with not seeing the Noudont civ trait, which I've replicated. Just posting what I'm doing here as I go. I think I already know the problem.

You can get to this civ's name conditions quickly by starting a game as Sidhe. Go at least 1 turn. Then:
Players[0]:ChangeJONSCulture(150)
That will get you 2 policy choices in next two turns. Select Tradition and then Folkart and they you can select Noudont civ.

They are supposed to get 1c from various resources, which is given by this:

UPDATE Traits SET FreeBuilding = 'BUILDING_1C_VARIOUS_RESOURCES' WHERE Type IN ('TRAIT_SAGUENAY', 'TRAIT_NOUDONT');

That's a hidden building. You can either use Fire Tuner to see if they have it, or save, exit, change this at the top of EaMain/EaDefines.lua
MapModData.bDebugShowHiddenBuildings = true
and then go back into the game.

So they aren't getting the FreeBuilding. Not when CivType changes and not after reload.

I'm checking possible DB problems now. But I suspect this is one case (of an unknown number) where a Trait effect isn't updating after a CivilizationType change during gameplay. Some do. Some don't. I didn't test all of the Trait effects that the mod uses. I'm afraid right now that FreeBuilding might be one that doesn't work. If this is true, then the fix will have to be in dll.

Here's the dll function that runs when a civ takes a name:

int CvLuaPlayer::lChangeCivilizationType(lua_State* L)
{
	CvPlayerAI* pkPlayer = GetInstance(L);
	const CivilizationTypes eNewCivType = (CivilizationTypes) lua_tointeger(L, 2);

	CvPreGame::setCivilization(pkPlayer->GetID(), eNewCivType);
	
	pkPlayer->GetPlayerTraits()->Reset();
	pkPlayer->GetPlayerTraits()->InitPlayerTraits();

	return 0;
}

Note: I'm not sure about this yet. I'll test another couple civ traits that use FreeBuilding so see if that (specifically) is the problem.

Edit: Tested for two other civs and the result in the same. FreeBuilding is not working when Trait added in game. When troubleshooting, note that all civs have two traits: one for race and one for civ name. In case it matters, the civ name Trait is after the race Trait in the DB. (It may be that the in-game gain isn't the problem, but rather it's having 2 traits that's the problem.)

 

[ls612]

I'm working on BrokenSky's problem with not seeing the Noudont civ trait, which I've replicated. Just posting what I'm doing here as I go. I think I already know the problem.

You can get to this civ's name conditions quickly by starting a game as Sidhe. Go at least 1 turn. Then:
Players[0]:ChangeJONSCulture(150)
That will get you 2 policy choices in next two turns. Select Tradition and then Folkart and they you can select Noudont civ.

They are supposed to get 1c from various resources, which is given by this:

UPDATE Traits SET FreeBuilding = 'BUILDING_1C_VARIOUS_RESOURCES' WHERE Type IN ('TRAIT_SAGUENAY', 'TRAIT_NOUDONT');

That's a hidden building. You can either use Fire Tuner to see if they have it, or save, exit, change this at the top of EaMain/EaDefines.lua
MapModData.bDebugShowHiddenBuildings = true
and then go back into the game.

So they aren't getting the FreeBuilding. Not when CivType changes and not after reload.

I'm checking possible DB problems now. But I suspect this is one case (of an unknown number) where a Trait effect isn't updating after a CivilizationType change during gameplay. Some do. Some don't. I didn't test all of the Trait effects that the mod uses. I'm afraid right now that FreeBuilding might be one that doesn't work. If this is true, then the fix will have to be in dll.

Here's the dll function that runs when a civ takes a name:

int CvLuaPlayer::lChangeCivilizationType(lua_State* L)
{
	CvPlayerAI* pkPlayer = GetInstance(L);
	const CivilizationTypes eNewCivType = (CivilizationTypes) lua_tointeger(L, 2);

	CvPreGame::setCivilization(pkPlayer->GetID(), eNewCivType);
	
	pkPlayer->GetPlayerTraits()->Reset();
	pkPlayer->GetPlayerTraits()->InitPlayerTraits();

	return 0;
}

Note: I'm not sure about this yet. I'll test another couple civ traits that use FreeBuilding so see if that (specifically) is the problem.

CvPlayerTraits::InitPlayerTraits() doesn't do anything about free buildings. I've implemented some test code which should persist and add free buildings from traits, which I can send to you for testing if you would like.

 

[Pazyryk]

Sure, just post code (or do a pull request). If it fixes problem I'll make a hotfix.

 

[ls612]

Sure, just post code (or do a pull request). If it fixes problem I'll make a hotfix.

Pull request sent. If that doesn't fix it (or if that trait is the only one which has problems) I'd suspect a DB error somewhere.