Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Message for the civ whose trade route got pirated #76

Merged
merged 1 commit into from Apr 9, 2019

Conversation

LynxAbraxas
Copy link
Contributor

Regarding #75, this PR implements that the victim gets a message about being pirated.
It does not include an implementation for removing the gold of that trade route (yet), because I'm not sure if that is taken into account at some place already.

@LynxAbraxas
Copy link
Contributor Author

This works as expected if tested as described in #75 (comment), so can go in on its own since the PR is not for fixing the general issues concerning piracy.

@LynxAbraxas LynxAbraxas changed the title WIP: message for the civ whose trade route got pirated Message for the civ whose trade route got pirated Apr 8, 2019
@MartinGuehmann
Copy link
Collaborator

Well, then let's put it in. You still have this from the intro:

It does not include an implementation for removing the gold of that trade route (yet), because I'm not sure if that is taken into account at some place already.

But I think it is taken into account at some place in the code I checked. So that issue should be addressed.

@MartinGuehmann MartinGuehmann merged commit c664d27 into civctp2:master Apr 9, 2019
@LynxAbraxas
Copy link
Contributor Author

@MartinGuehmann many thanks for merging and pointing out the missing part from the intro, had forgotten about that.
Where did you find it in the code you checked? I was not able to find that so far only the place where it is added to the attacker

g_player[m_owner]->AddGold(pgold);

and I would expect the subtraction close by.

It is also not clear to me if multiple armies/units are pirating the same trade route all of them get the gold (which would be inappropriate) and if it subtracted multiple times (which would be inappropriate even more), which might be related to #75.

@LynxAbraxas LynxAbraxas deleted the gotPiratedMessage branch April 10, 2019 20:10
@MartinGuehmann
Copy link
Collaborator

Where did you find it in the code you checked? I was not able to find that so far only the place where it is added to the attacker

For the sake, I just assumed but it is in CityData.cpp

sint32 CityData::CalculateGoldFromResources()
{
	m_goldFromTradeRoutes = 0;
	m_goldLostToPiracy    = 0;

	for(sint32 i = 0; i < m_tradeSourceList.Num(); i++)
	{
		if(!m_tradeSourceList[i]->IsBeingPirated())
		{
			m_goldFromTradeRoutes += m_tradeSourceList[i]->GetValue();
		}
		else
		{
			m_goldLostToPiracy += m_tradeSourceList[i]->GetValue();
		}
	}

	sint32 wonderTradeBonus = wonderutil_GetMultiplyTradeRoutes(m_builtWonders);

	if(wonderTradeBonus > 0)
	{
		m_goldFromTradeRoutes += sint32(double(m_goldFromTradeRoutes) *
										(double(wonderTradeBonus) * 0.01));
		m_goldLostToPiracy += sint32(double(m_goldLostToPiracy) *
										(double(wonderTradeBonus) * 0.01));
	}

	return m_goldFromTradeRoutes;
}

Actually, you don't "subtract" you just "do not add". However, m_goldLostToPiracy is just calculated but not used, actually it should show up in the trade manager under advice. So if you want to add a message it should go to the pirate event and the loss shall be the value of the trade route. Obviously, that is quite obscure.

civctp2/ctp2_code/gs/gameobj/ArmyData.cpp

Line 1660 in e0edb94

g_player[m_owner]->AddGold(pgold);

and I would expect the subtraction close by.

Nope, as you can see it is somewhere else, and not even a subtraction.

It is also not clear to me if multiple armies/units are pirating the same trade route all of them get the gold (which would be inappropriate) and if it subtracted multiple times (which would be inappropriate even more), which might be related to #75.

That's easy you already cited that code:

if(route->GetPiratingArmy().m_id == m_id) {

When you pirate then the pirating army is set. Let's say player 1 pirates. Then on your next turn you get the gold, and the victim does not earn anything from the trade route. Now after your turn, player 2 pirates the same trade then the pirating army is a different army, not yours but of player 2. After player 2 eventually, it will be your turn again, then you won't get any gold and if you don't pirate the route again then player 2 gets on his turn the gold. But if you pirate the route again then the pirating army is your and so player 2 won't get any gold.

So if two players are pirating the same trade route over turns then nobody gets the bounty. Of course, still the trade route does not give any gold to the owner.

@LynxAbraxas
Copy link
Contributor Author

Nope, as you can see it is somewhere else, and not even a subtraction.

Many thanks @MartinGuehmann for pointing that out. The fact that it is not a subtraction is the reason that I did not find it (search through all SubGold).

So if two players are pirating the same trade route over turns then nobody gets the bounty. Of course, still the trade route does not give any gold to the owner.

Many thanks for that explanation. I think this is likely the reason for the effect that is bugging me, and which I think should be improved, but I'm not sure how so far.

And the explanation above is probably the reason why the message for the civ whose trade route got pirated is not show, and also why the trade manager does not show the pirating square and also why the diplomatic window does not expose the option to ask for "stop pirating" even if you see that player pirating your routes.

I've a test game ready, just need to write up the interaction to reproduce the problem, will try to get to that tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants