OpenXcom / OpenXcom

Open-source clone of the original X-Com 👽
https://openxcom.org
GNU General Public License v3.0
1.91k stars 486 forks source link

Potential error in country funding calculation #1244

Closed Player701 closed 4 years ago

Player701 commented 4 years ago

I first mentioned this on the forum a few years ago as issue no. 2, but no developer has responded since. Guess I should have reported this as a bug right away.

Synopsis: There is the following comment in MonthlyReportState.cpp:

https://github.com/OpenXcom/OpenXcom/blob/a8080d499c811d56b44c8eb5cc1dced871e27d03/src/Geoscape/MonthlyReportState.cpp#L348-L349

As far as I understand it, this means that the score earned for completing research projects should not play a part in determining funding changes of countries. However, this comment and the actual code seem to disagree with each other. This is the part where funding amounts are updated:

https://github.com/OpenXcom/OpenXcom/blob/a8080d499c811d56b44c8eb5cc1dced871e27d03/src/Geoscape/MonthlyReportState.cpp#L368-L395

To be precise, this is the exact line where it happens:

https://github.com/OpenXcom/OpenXcom/blob/a8080d499c811d56b44c8eb5cc1dced871e27d03/src/Geoscape/MonthlyReportState.cpp#L380

And now look at how xcomTotal is calculated:

https://github.com/OpenXcom/OpenXcom/blob/a8080d499c811d56b44c8eb5cc1dced871e27d03/src/Geoscape/MonthlyReportState.cpp#L355

As we can see, it includes the research score. This value is passed to each country to influence its funding change, which contradicts the aforementioned comment (lines 348-349). Depending on whether or not this code is correct, one of the following should be done:

IMO, it doesn't really make sense to have research influence country funding, but I haven't done any reverse engineering on vanilla XCOM for that matter - maybe it really does work that way?

Warboy1982 commented 4 years ago

the comment is misleading, but a code dive indicates that we're in-line with vanilla behaviour

Player701 commented 4 years ago

Thank you for clarifying this. If possible, please remove the comment then, so that other people working with the code don't get confused, like I was.