API change: Get quarterly company information.

[Morloth]

Hey guys,

I read through the API change topic http://www.tt-forums.net/viewtopic.php?f=65&t=44249 and noticed someone asked for a way to get to the quarterly company information. Since I also need this information for the next version of my AI I decided to implement these features. The given patch adds the following functions:

Money GetQuarterlyIncome(CompanyID company, uint32 quarter);
Money GetQuarterlyExpenses(CompanyID company, uint32 quarter);
int32 GetQuarterlyCargoDelivered(CompanyID company, uint32 quarter);
int32 GetQuarterlyPerformanceRating(CompanyID company, uint32 quarter);
Money GetQuarterlyCompanyValue(CompanyID company, uint32 quarter);

if quarter is equal to 0 (AICompany::CURRENT_QUARTER) it will display the current value (except for the Performance Rating, because it is calculated at the end of every quarter).

I have not updated the regression test, but I'll do so once people are happy with the code provided. Note that int32 GetQuarterlyPerformanceRating(CompanyID company, uint32 quarter); makes AICompany::GetPerformanceRating(CompanyID company) redundant - I haven't removed it but if this patch gets accepted I suspect the latter will eventually be phased out.

Hope it gets included .
Bram

[planetmaker]

I think that

Code: Select all

+	 * @pre ResolveCompanyID(company) != COMPANY_INVALID.

is actually not true. You handle every CompanyID, including COMPANY_INVALID within your code. Even though current code doesn't always follow it: the doxygen comments / documentation should be near the actual code, not in the header files (i.e. doxygen code goes in ai_company.cpp).

And... I think adding regression test update also initially doesn't hurt It's easily forgotten.

[Morloth]

Hi planetmaker, thanks for your comments.

I think that

Code: Select all

+	 * @pre ResolveCompanyID(company) != COMPANY_INVALID.

is actually not true. You handle every CompanyID, including COMPANY_INVALID within your code. Even though current code doesn't always follow it: the doxygen comments / documentation should be near the actual code, not in the header files (i.e. doxygen code goes in ai_company.cpp).

I don't see any doxygen comments in the actual implementation - only in the declarations. I'm quite sure they should be in the header files. Also the @pre should be included, if you look at the other functions they also state the same requirement but handle an invalid company in the same way.

And... I think adding regression test update also initially doesn't hurt It's easily forgotten.

Very true.

[planetmaker]

Even though current code doesn't always follow it: the doxygen comments / documentation should be near the actual code, not in the header files (i.e. doxygen code goes in ai_company.cpp).

I don't see any doxygen comments in the actual implementation

No need to continue the "wrong" way.

The documentation should be as close to the actual code as possible to maximise the chance of staying in sync.
Comments for functions go in the .cpp file.
Comments for inlines go in the .h/.hpp file.

[Yexo]

Planetmaker: in this particular case Morloth is correct by putting the documentation in the header file. The AI API has all function-level documentation in the header files and the code in the cpp files. The cpp files only have some lines of documentation where it's needed to clarity the code.

Edit: haven't looked at your patch yet, but I'll take a look tomorrow.

[Morloth]

Updated patch, included the regression test.

[Yexo]

I'd write the functions like this, checking all preconditions first, returning results later.

Code: Select all

/* static */ Money AICompany::GetQuarterlyIncome(AICompany::CompanyID company, uint32 quarter)
{
	company = ResolveCompanyID(company);
	if (company == COMPANY_INVALID) return -1;
	if (quarter > EARLIEST_QUARTER) return -1;

	if (quarter == CURRENT_QUARTER) {
		return ::Company::Get((::CompanyID)company)->cur_economy.income;
	}
	return ::Company::Get((::CompanyID)company)->old_economy[quarter - 1].income;
}

Note also the cast to (::CompanyID) instead of (CompanyID). This is to make sure we cast to the globally defined CompanyID, not to AICompany::CompanyID.

(except for the Performance Rating, because it is calculated at the end of every quarter).

Please make this an explicit precondition and check for it.

Note that int32 GetQuarterlyPerformanceRating(CompanyID company, uint32 quarter); makes AICompany::GetPerformanceRating(CompanyID company) redundant - I haven't removed it but if this patch gets accepted I suspect the latter will eventually be phased out.

I can't find AICompany::GetPerformanceRating at all. Any hints to where I can find it?

Hope it gets included .

The patch looks good, so if you could fix the small points above it certainly will.

[Morloth]

I'd write the functions like this, checking all preconditions first, returning results later.

Code: Select all

/* static */ Money AICompany::GetQuarterlyIncome(AICompany::CompanyID company, uint32 quarter)
{
	company = ResolveCompanyID(company);
	if (company == COMPANY_INVALID) return -1;
	if (quarter > EARLIEST_QUARTER) return -1;

	if (quarter == CURRENT_QUARTER) {
		return ::Company::Get((::CompanyID)company)->cur_economy.income;
	}
	return ::Company::Get((::CompanyID)company)->old_economy[quarter - 1].income;
}

Note also the cast to (::CompanyID) instead of (CompanyID). This is to make sure we cast to the globally defined CompanyID, not to AICompany::CompanyID.

Are you sute about this? Because other functions like GetCompanyValue, GetBankBallance, and AutoRenew* have the same cast I used in my patch. Would you like me to change those too?

(except for the Performance Rating, because it is calculated at the end of every quarter).

Please make this an explicit precondition and check for it.

Done.

Note that int32 GetQuarterlyPerformanceRating(CompanyID company, uint32 quarter); makes AICompany::GetPerformanceRating(CompanyID company) redundant - I haven't removed it but if this patch gets accepted I suspect the latter will eventually be phased out.

I can't find AICompany::GetPerformanceRating at all. Any hints to where I can find it?

Sorry I meant AICompany::GetCompanyValue.

Hope it gets included .

The patch looks good, so if you could fix the small points above it certainly will.

Sweet! Please find the new patch in the first post (v3).

[Yexo]

Note also the cast to (::CompanyID) instead of (CompanyID). This is to make sure we cast to the globally defined CompanyID, not to AICompany::CompanyID.

Are you sute about this? Because other functions like GetCompanyValue, GetBankBallance, and AutoRenew* have the same cast I used in my patch. Would you like me to change those too?

Hmm, the current code is already inconsistent there, I didn't see that at first. Leave it however it is now in your patch and it can be made consistent later.

Note that int32 GetQuarterlyPerformanceRating(CompanyID company, uint32 quarter); makes AICompany::GetPerformanceRating(CompanyID company) redundant - I haven't removed it but if this patch gets accepted I suspect the latter will eventually be phased out.

I can't find AICompany::GetPerformanceRating at all. Any hints to where I can find it?

Sorry I meant AICompany::GetCompanyValue.[/quote]Since this patch makes GetCompanyValue obsolete, please remove it completely. In bin/ai/compat_0.7.nut (and 1.0 / 1.1, but not 1.2) add the following code:

Code: Select all

AICompany.GetCompanyValue <- function(company)
{
	return AICompany.GetQuarterlyCompanyValue(company, AICompany.QUARTER_CURRENT);
}

Also note this change in ai_changelog.hpp and adjust the regression test. After that I'd say your patch is done

[Morloth]

Updated the patch to v4, thanks for your feedback .

[Yexo]

I added the changes to ai_changelog.hpp and committed your patch in r22584. Thanks for a very nice patch

[Morloth]

Thank you very much for including it!

[Zuu]

Thanks for the work with this patch. I think it is useful for the current (but stalled) work with PAXLink where i manually tried to compute the income and profit.