Messages

[Spoiler]

16

Suggestions / Monthly rating / statistics issues

« on: August 14, 2018, 02:27:08 pm »

UPD: this should probably be moved to "Contributions->Programming". Sorry, I was lazy to scroll the forums list down a little. 

I have found several issues with how some of the values in the statistics window (displayed at the end of the campaign) are calculated, as well as with the monthly rating calculation code. I would like to provide a breakdown of all issues discovered as well as to propose and discuss solutions to them. Please correct me if some of the assumptions I make here are wrong; since I have only worked with the source code for a short time, it is difficult to maintain a consistent picture of all things in my head.

I'm sorry making for such a long post; I thought it would be better to collect everything in one place because the affected parts of the code are closely related. I've added some spoiler tags to reduce the visible length of the post. I hope the developers can look into it when they have the time. I can also provide pull requests to implement the solutions proposed here. In the end, I hope we all can make OpenXcom better together

The first group of issues is related to MonthlyReportState.cpp, in particular, to MonthlyReportState::calculateChanges.

#1. Questionable piece of code in MonthlyReportState::calculateChanges
Description:

Spoiler:

The last two lines in this piece of code do not seem to have any purpose (Source: MonthlyReportState.cpp, line 335):

Code: [Select]

int lastMonthOffset = _game->getSavedGame()->getFundsList().size() - 3;
if (lastMonthOffset < 0)
lastMonthOffset += 2;

Here the game calculates the index to use in vectors which store the values necessary to compute the last month's rating. If this index ends up being negative, then 2 is added to the resulting value. Since the value returned by getFundsList() contains at least 2 values at this time (it is populated by one value at the start of the game and then with another value just before MonthlyReportState::calculateChanges is called), the check "lastMonthOffset < 0" is practically equivalent to "lastMonthOffset == -1". Therefore, lastMonthOffset will be equal to -1 + 2 = 1 if getFundsList() is 2 values long, which happens only at the end of the first game month.

However, the actual computation logic for the last month's rating is guarded by additional checks on the corresponding vectors' sizes:

When accounting for XCOM and alien activities (Source: MonthlyReportState.cpp, line 343):

Code: [Select]

if ((*k)->getActivityXcom().size() > 2)
_lastMonthsRating += (*k)->getActivityXcom().at(lastMonthOffset)-(*k)->getActivityAlien().at(lastMonthOffset);

When adding the research score (Source: MonthlyReportState.cpp, line 357):

Code: [Select]

if (_game->getSavedGame()->getResearchScores().size() > 2)
_lastMonthsRating += _game->getSavedGame()->getResearchScores().at(lastMonthOffset);

All these vectors - funds, XCOM activities, alien activities, and research scores - have the same size, because they are updated more or less at the same time (funds and research scores are updated in SavedGame::monthlyFunding, the other two are updated in MonthlyReportState::calculateChanges just before the rating is calculated). Therefore, adding 2 to lastMonthOffset if it ends up being negative does not serve any purpose at all - it will only end up negative if the size is 2, and if that is true, the "if" branches in the last two snippets will not be executed, effectively leaving the last month's rating at 0.

Proposed solution:

Spoiler:

Depends on what exactly should the last month's rating be equal to after the first month. In both cases, the last two lines in the first code snippet should be removed.

If the last month's rating at the end of the first month should be the same as the current month's rating, then calculate lastMonthOffset as follows:

Code: [Select]

int lastMonthOffset = std::max(0, _game->getSavedGame()->getFundsList().size() - 3);

Also, both "if" checks in the last two snippets above should be removed.

If the last month's rating at the end of the first month should be 0 (current behavior), then replace both "if" conditions in the last two snippets above with "lastMonthOffset >= 0".

#2. Possible oversight in MonthlyReportState::calculateChanges
Description:

Spoiler:

There is a comment in MonthlyReportState::calculateChanges which says (Source: MonthlyReportState, line 348):

Code: [Select]

// apply research bonus AFTER calculating our total, because this bonus applies to the council ONLY,
// and shouldn't influence each country's decision.

However, what the comment says seem to contradict what the code below actually does. Specifically, it passes the value of xcomTotal in a loop to Country::newMonth when sending the rating information to all countries (Source: MonthlyReportState, line 380):

Code: [Select]

(*k)->newMonth(xcomTotal, alienTotal, pactScore);

And note how xcomTotal is calculated (Source: MonthlyReportState, line 355):

Code: [Select]

xcomTotal = _game->getSavedGame()->getResearchScores().at(monthOffset) + xcomSubTotal;

xcomTotal includes the research score, which is in turn passed to all countries. However, the comment above says that the research bonus applies to the council only, and shouldn't influence each country's decision. This is probably an oversight.

Proposed solution:

Spoiler:

If what the code does is actually wrong, then pass the value of xcomSubTotal instead of xcomTotal when updating the data for the countries.

The next set of issues is related to the statistics window, which shows various statistical information to the player after the game has ended.

#3. Average monthly rating is incorrectly calculated based on research scores only

Description: See issue #1409 on the bug tracker.

Proposed solution: depends on the solution to the following issue #4. See below.

#4. Consequences of the limit on the number of values stored in certain vectors in the save file
Description:

Spoiler:

The total income and expenditure values are based on the data for the last 12 months only. This is because of how many individual values are stored in the save file, which is at most 12. This behavior is not a bug per se (it seems to be very much intended according to the comments in the code), but if the campaign has lasted for more than 12 months, then summing these values up will not result in the correct totals for the whole campaign. The same issue applies to various other values, including but not limited to: funds, research scores and XCOM and alien activity in regions and countries.

Proposed solution:

Spoiler:

There are at least two possible ways to solve this as well as #3.

• Store cumulative values for income, expenditure and monthly score in the save file. These values can be updated in MonthlyReportState::calculateChanges (monthly score) and / or SavedGame::monthlyFunding (income and expenditure). It is probably most consistent to update the corresponding cumulative value at the same time when calculating / pushing the normal value. Then, when calculating total income and total expenditure, the resulting cumulative values can be used. The current month should also be accounted for, since the cumulative values are only updated at the end of a game month. This means that for incomes / expenditures we need to also add the last values from the corresponding vectors to the respective cumulative values, and for the monthly score we have to calculate it based on the current research score as well as XCOM and alien activities in the current month.
• Lift the limit of 12 values to store in the corresponding vectors. For consistency, it would probably be best to lift this limit on all vectors where it is currently present. This will also necessitate some changes in GraphState.cpp so that only the last 12 values are used when calculating graph data. Then, when calculating total income and expenditure, no change of the current code is required. However, the monthly rating calculation will still need fixing to account for XCOM an alien activities in addition to research scores.

#5. Unused values in StatisticsState.cpp
Description: There are at least two values that are calculated but not used when displaying the statistics window. These are, respectively, the number of alien bases destroyed and the number of XCOM bases lost. They are calculated when iterating over the mission statistics vector (Source: StatisticsState.cpp, line 147):

Code: [Select]

if ((*i)->isAlienBase() && (*i)->success)
{
alienBasesDestroyed++;
}
if ((*i)->isBaseDefense() && !(*i)->success)
{
xcomBasesLost++;
}

Proposed solution: pull request.

17

OpenXcom Extended / Re: [OXCE+] Original discussion thread

« on: August 11, 2018, 05:17:49 pm »

Hi again.

There seems to be a visual glitch where a hovertank's turret is incorrectly drawn behind its chassis. I remember encountering this glitch in vanilla OXC two years ago (even reported it on the bug tracker) but it doesn't seem to happen there anymore. OXCE+ is still affected, though.

Here are a couple save files to demonstrate the glitch. Press the "View Level Above" button or use the mouse wheel and you will see it. Also attaching screenshots.

18

Open Feedback / Re: UFO_170.map and the universal patch

« on: August 10, 2018, 09:27:05 pm »

Well, since no one has bothered to check, here is a detailed comparison of the two files.

You can see the difference on the first screenshot. The version from the Extras patch has byte 00 occupy the offset 1A0B, while hellrazor's version has byte 19 at this offset. This byte corresponds to the "ground" value of the 17th tile in the 16th row on the 2nd level of the map. With the MapView tool it can be seen that the ground is indeed missing in this particular tile in the Extras version of the map - which is exactly the point from where I can spot targets below and shoot at them (of course, the aliens have this advantage most of the time, but I've run a few tests in debug mode as well).

There is also an extra byte at the end of the file in hellrazor's version, making it 9604 bytes long, which is one byte longer than the intended value of 9603 (because 3 bytes in the beginning specify the dimensions of the map and the remaining part is composed of 4-byte records). I guess this extra byte is simply ignored by OpenXcom, so it doesn't really make much difference - although it makes it possible to simply compare how long the files are to refute the claim that hellrazor's version is the one which comes with the universal patch (because it is not).

Perhaps there is a new version of the patch somewhere which is not that easy to find? Similar to how "1.0" is posted as the latest release on the main page and the nightlies are a bit harder to discover for the uninitiated.

19

Released Mods / Re: [TFTD] Combo Patch v1.8 (updated September, 10th)

« on: August 06, 2018, 01:53:33 pm »

Just to clarify, my intention was not to provide any further updates but rather analyze how well the Combo Patch and the Extras patch mixed together. However, if I encounter some kind of glitch in a map during my playthroughs, I will see what I can do. Not sure how easy it is to find any, though - considering that these patches have existed for quite some time by now and received several updates since their first releases. I'm also not very experienced in working with the game files, and know little about various tools which can be used to edit them (UFOpaedia has been of immense help though).

20

OpenXcom Extended / Re: [OXCE+] Original discussion thread

« on: August 05, 2018, 07:49:23 pm »

I have made a partial fix for OXCE+ (which had a bit more stuff in the destructor)... it's now only as bad as in OXC.

OK, I've filed a bug report so that OXC developers don't foget about it.

This is a known OXCE issue... 2+ years old.
Yankes doesn't seem to be very interested in fixing it; or forgot about it.

Reason: the sprite ID = -1, which is normally a default value meaning "not set", but in this case it is unfortunately a valid index.

I'll fix it in OXCE+.

I see. Thank you then

21

Released Mods / Re: [TFTD] Combo Patch v1.8 (updated September, 10th)

« on: August 05, 2018, 05:22:29 pm »

Hi. Thank you very much for this data patch.

I also wanted to use the patch from the extras section of the downloads page, and it appears that there are several files which are present both in that patch and this combo patch. Most of them are identical save for these three ones: ENTRY03.MAP, GRUNGE08.MAP and GRUNGE09.MAP. I decided to inspect the differences in the files and post the results of the analysis here.

• ENTRY03.MAP: Compared to the file from Extras, the file from the Combo Patch adds a "railing" object to the second level of one of the "tower" structures located on the seabed level of Alien Colonies, and also removes a wall looking towards the second "tower" (which is connected to the first one by a corridor). This looks like a viable change but it also seems to break the symmetry with the opposite "tower". The Extras patch doesn't have the "railing" and the wall is present in the proper place (compared to its misplaced position 1 tile to the north in the original version). See the screenshot for a clearer explanation.
• GRUNGE08.MAP: The Combo Patch version additionally removes an object from a tile (see screenshot). This object doesn't seem to block any paths, but could probably prevent stepping on this tile (I haven't checked myself).
• GRUNGE09.MAP: Similar to GRUNGE08.MAP, there is an extra object placed on one of the tiles which may prevent stepping on it but does not block paths. This time, however, the extra object is removed in the Extras patch but is present in the Combo Patch.

TL;DR: The differences between the files are quite minor and it is safe to install one patch on top of the other in any order. However, for OCD people like myself, I'd recommend using the following versions of the files: ENTRY03.MAP from the Extras patch, GRUNGE08.MAP from the Combo Patch, and GRUNGE09.MAP from the Extras patch again. Though, again, the differences are so minor most players won't probably notice.

I'm attaching an archive which contains files from both patches combined; the three files which differ between the patches have been selected according to my comment above. I do not take any credit for this work and respect the authors of the original patches. Also, kudos to whoever made the MapView tool which I used to analyze the files.

22

OpenXcom Extended / Re: [OXCE+] Original discussion thread

« on: August 05, 2018, 12:46:48 pm »

Yeah, trying to access already deleted objects... also applies to vanilla.

I see. Hope it can be fixed in future vanilla and OXCE+ builds, then. Maybe file a report on the bug tracker for that one?

Here is another, this time OXCE+ exclusive issue (maybe I should create separate threads for them?): Chryssalid's "built-in" weapon has no sprite in the status panel (viewable when mind controlled), while in vanilla OXC it does have one. Not sure why, as the corresponding resource is present and rulesets appear to be identical. All other "built-in" weapons (Celatid, Silacoid, Reaper, Zombie) have their sprites correctly displayed. See the attached screenshot and save file.

23

Open Feedback / UFO_170.map and the universal patch

« on: August 05, 2018, 10:50:07 am »

Hi.

I've been playing XCOM 1 using OXCE+ and sometimes there have been cases where aliens on the second floor of the Supply Ship UFO can see my soldiers down below and shoot through the wall / floor. I thought it was an engine bug at first but then I found this and eventually it led me to this thread where people say the issue is fixed in the universal data patch provided here: https://openxcom.org/download/extras/universal-patch.zip. However, I have the patch installed and the issue is still present.

To quote the aforementioned thread, user hellrazor posted an attachment with the fixed UFO_170.map file and user Tarvis said:

Well, I don't know what to tell you. hellrazor's modified map file is identical to the one in the universal patch.

I just checked and this is not true. Even the size of the two files differs (attached is 9604 bytes long, the one which comes with the patch is 9603 bytes long). hellrazor's attached file does indeed seem to patch the "hole" in the UFO. If possible, could the maintainers of the patch update it with the fixed file? Thank you very much.

24

OpenXcom Extended / Re: [OXCE+] Original discussion thread

« on: August 03, 2018, 10:53:13 pm »

Hi again.

Found another bug in the latest build (2018-07-29).

Steps to reproduce:

• Load save1.sav
• Intercept the UFO with any craft (they are all armed), DO NOT SHOOT THE UFO DOWN, minimize the interception window instead
• Load save2.sav
• BOOM - segmentation fault

Unfortunately, these steps are not reliable (the crash doesn't always happen). But it appears the memory becomes corrupted after performing steps 1-3 and any further action taken may result in a crash or freeze. I had OXCE+ lock up completely when trying to load save2.sav again after it loaded normally for the first time. In another attempt to reproduce the bug, I had OXCE+ crash when exiting the game. I couldn't reproduce this in vanilla OpenXcom (though it might be subject to this bug as well, considering the apparent randomness in how it can manifest itself).

Tested on a clean installation - no mods used and no mods present. OS is Windows 10 x64, using build from the URL in the corresponding forum post (win32). BTW, the win64 build doesn't seem to work at all on my system (it just doesn't start, no error message or anything).

25

Open Feedback / Re: MISSING @MORE THAN 100% CHANCE

« on: August 01, 2018, 07:02:55 am »

I see... well, 50% is much more than nothing. Thank you for wasting time on this, I guess

26

Open Feedback / Re: MISSING @MORE THAN 100% CHANCE

« on: July 31, 2018, 09:56:35 pm »

Try turning on save scumming... I think after a few reloads, it will hit... RNG man

Hmm. I actually do have it on most of the time, and now that you mentioned it, I've tested it some more, and aimed shots actually do hit sometimes. For some reason, though, it feels as if auto shots hit much more often - though it might just be my imagination at play. I don't think I have the patience to test it enough times for the results to be statistically significant...

27

Open Feedback / Re: MISSING @MORE THAN 100% CHANCE

« on: July 31, 2018, 09:22:31 pm »

Wow. Just... wow.

Then, I guess, the question is: why doesn't the trajectory go through the target? Would it hit something in between the shooter and the target if it did? If this is the case, then why does the game think there is a line of fire at all?

28

OpenXcom Extended / Re: [OXCE+] Original discussion thread

« on: July 30, 2018, 06:55:17 pm »

There seems to be a minor regression issue between builds 2018-07-29 and 2018-07-26. The weapon sprite in the status panel seems to disappear randomly while firing. Note that it has always done that once after clicking on the target (just before firing), but in that case everything in the panel disappears also (soldier stats, name etc.). In 2018-07-29 it seems to disappear randomly during the firing sequence, leaving only the ammo counter visible. This is most noticeable with auto shots.

29

Open Feedback / Re: MISSING @MORE THAN 100% CHANCE

« on: July 30, 2018, 06:12:21 pm »

From what I've just read above, I can understand the "%" is misleading. However, I still can't believe the following case is not a bug... try out the attached save file (using OXCE+ build 2018-07-29). Aimed shots seem to pass right through the floater, as if he were a ghost or something. I think I've had it happen before with aliens standing in the exact same position (one square away from an abductor's entrance hatch) and trying to hit them from the side. This simply doesn't look like it can't be a bug... from a (mostly) casual player's point of view, at least.

30

OpenXcom Extended / Re: [OXCE+] Original discussion thread

« on: July 26, 2018, 07:48:38 pm »

Yes.

This could be easily rephrased, if it's bothering you.
I left it as in original, because it's not bothering me

It would be one more dimension to an already overcomplicated situation (and I didn't even tell you there is an unlimited number of alien containment types for various groups of creatures... by far my least favourite oxce+ feature).
I only hope that benefits from having after-mission-transfers (for other players) outweigh your non-vanilla frustration.

There is no frustration, just expressing my thoughts. Like I said, it might be easier to simply not use this feature instead. Especially since you say the situation is overcomplicated (well, I think I already got that when looking at the code and comparing it to plain OpenXcom). Maybe I will even try out one of those mods someday which make use of different alien containment types