aliens

Author Topic: [OLD] Old OXCE discussion thread  (Read 778963 times)

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1335 on: July 09, 2017, 06:15:35 am »
Right, fast fix is to not throw in `getActionConf`, but what point is then checking for ammo in that weapon then if always it will return `null`?
The point is that it allows you to ask for the ammo type without having to first check if the action is an attack, if the attack needs a weapon, if the weapon is loaded, and whatever else comes up. Apparently the AI code in this particular case just wants to know if their weapon attack power is above some arbitrary threshold to overrule their decision to use psi. If there is no ammo, that's fine - it just means that the attack power is not above the threshold, because there is no attack. It certainly isn't a case where we'd want to throw an exception. And I imagine there could be many other similar situation. There is no problem with getAmmoForAction returning null when there is no ammo associated with the action. That's exactly what I'd expect it to do, and it's a meaningful return value.

[edit]
By the way, if you are trying to catch logic bugs, I suggest that using something similar to `assert` would be nicer than using `throw`. As I player, I don't mind being warned that something unexpected has happened - but it is very annoying to lose playtime because of it. I mean, I think it would be nicer if there was a message saying that something has gone wrong without the game immediately crashing.
« Last Edit: July 09, 2017, 08:16:20 am by karadoc »

Offline Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 9091
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1336 on: July 09, 2017, 08:40:34 am »
Proper fix would be check correct action type (aim/auto/snap/hit) otherwise we could remove this check completely.

"Launch" is probably also a valid action type...

Btw. are you going to publish some hotfixes in near future? Solarius (against recommendation) used 3.8 version in xcf and it's crashing a lot.

Offline Solarius Scorch

  • Global Moderator
  • Commander
  • *****
  • Posts: 11722
  • WE MUST DISSENT
    • View Profile
    • Nocturmal Productions modding studio website
Re: [OXCE] OpenXcom Extended
« Reply #1337 on: July 09, 2017, 10:44:21 am »
Solarius (against recommendation) used 3.8 version in xcf and it's crashing a lot.

Er, no, I haven't. Some players upgraded, though.

Offline Yankes

  • Global Moderator
  • Commander
  • ***
  • Posts: 3350
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1338 on: July 09, 2017, 01:02:03 pm »
The point is that it allows you to ask for the ammo type without having to first check if the action is an attack, if the attack needs a weapon, if the weapon is loaded, and whatever else comes up.
For that I added for Meridian new function in next version `needAmmoForAction` that have wide contract and can accept any action type and any item.

Quote
Apparently the AI code in this particular case just wants to know if their weapon attack power is above some arbitrary threshold to overrule their decision to use psi. If there is no ammo, that's fine - it just means that the attack power is not above the threshold, because there is no attack. It certainly isn't a case where we'd want to throw an exception. And I imagine there could be many other similar situation. There is no problem with getAmmoForAction returning null when there is no ammo associated with the action. That's exactly what I'd expect it to do, and it's a meaningful return value.
Problem is that in this case it will never return any ammo, because action type is always rethink. This line is not checking what it suppose to check. Returning null will only hide bug that I introduce there.

Quote
[edit]
By the way, if you are trying to catch logic bugs, I suggest that using something similar to `assert` would be nicer than using `throw`. As I player, I don't mind being warned that something unexpected has happened - but it is very annoying to lose playtime because of it. I mean, I think it would be nicer if there was a message saying that something has gone wrong without the game immediately crashing.
`assert` terminate program too (if is enable, otherwise will not do anything). You are right that `throw` is annoying for player but flip side is that if error would be only logged then user will never report it because game run "fine".

I will think about better solution that will not f*** up user and force him to report bugs, right now I think about big read text on top of screen. This will be annoying enough to force player to report bug but will not interrupt his game.

"Launch" is probably also a valid action type...

Btw. are you going to publish some hotfixes in near future? Solarius (against recommendation) used 3.8 version in xcf and it's crashing a lot.
And it is supported like other ones.

And for new version, it will be released today or tomorrow, it will be with new goodies from master and fix for all reported bugs.

Offline Yankes

  • Global Moderator
  • Commander
  • ***
  • Posts: 3350
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1339 on: July 11, 2017, 01:57:22 am »
New version is ready, primary change is merge last changes from master and bug fix.
New functionality is script that can alter exp grain (as multiplier on top of Meridian system).

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1340 on: July 11, 2017, 11:06:00 am »
From what I can tell, you haven't included a fix for the crash we were just discussing; the one fixed by this.

For the particular save file that I uploaded, it was faulty AI which caused the crash - as you pointed out - but nevertheless, the patch that I've posted would prevent this crash and potentially prevent other similar crashes. The patch makes the behaviour of `getAmmoForAction`consistent with the new `needsAmmoForAction` function, in that neither should crash if the action does not have rules set.

Of course, you'll probably want to fix the AI problem as well. If you use my patch, then you can just leave the AI alone and fix it later. Alternatively, if you don't like my patch, you could just delete the problematic block from the AI and it will make no difference (other than avoiding the crash).

Here's the relevant bit of code
Code: [Select]
if (_visibleEnemies)
{
auto ammo = _attackAction->weapon->getAmmoForAction(_attackAction->type);
if (ammo && ammo->getRules()->getPowerBonus(_attackAction->actor) >= weightToAttack)
{
return false;
}
}
else if (RNG::generate(35, 155) >= weightToAttack)
{
return false;
}
If you delete that inner if block, that will avoid the crash. (That condition will never be true anyway, because the attack action never has ammo.) Perhaps it would be even better to delete the entire `_visibleEnemies` block though, just leaving the final random number test.

--

In other news, I've been making some tweaks to how items are arranged on the ground. Check out the attacked picture comparing the current unorganised equipment pile vs the sorted equipment pile from my code. I'll upload a patch for that soon if you have interest in it.

Offline Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 9091
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1341 on: July 11, 2017, 12:13:31 pm »
One more similar, but different bug that needs a fix: https://openxcom.org/forum/index.php/topic,5047.msg84804.html#msg84804

Quote
A fatal error has occurred: Unsupported action (val: 12) for item STR_DOGE_TRACK

Notice val: 12 (BA_LAUNCH), not 8 (BA_SNAPSHOT) as in the earlier bug

I will add that this bug was also not fixed yet... still crashes.

Offline Yankes

  • Global Moderator
  • Commander
  • ***
  • Posts: 3350
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1342 on: July 11, 2017, 09:17:32 pm »
Here's the relevant bit of code
Code: [Select]
if (_visibleEnemies)
{
auto ammo = _attackAction->weapon->getAmmoForAction(_attackAction->type);
if (ammo && ammo->getRules()->getPowerBonus(_attackAction->actor) >= weightToAttack)
{
return false;
}
}
else if (RNG::generate(35, 155) >= weightToAttack)
{
return false;
}
If you delete that inner if block, that will avoid the crash. (That condition will never be true anyway, because the attack action never has ammo.) Perhaps it would be even better to delete the entire `_visibleEnemies` block though, just leaving the final random number test.

sh*** I forget to fix it, and again you do not fix this but, you remove this check because Psi is always check before Weapons and this cause that `_attackAction->type`do not have any valid attack type. This was my whole point.

Notice val: 12 (BA_LAUNCH), not 8 (BA_SNAPSHOT) as in the earlier bug

I will add that this bug was also not fixed yet... still crashes.
Failed again :/ I focus too much on "NULL" bug :/

I will release today fixed version.

Offline Yankes

  • Global Moderator
  • Commander
  • ***
  • Posts: 3350
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1343 on: July 11, 2017, 11:29:20 pm »
Missed bugs are fix.

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1344 on: July 12, 2017, 01:51:01 am »
Here is my patch for improving the order of items on the ground.

https://github.com/karadoc/OpenXcom/commit/c446001e3f13f1be0352fd9bf3e6b307a71ee0d4

Note that most of the changes in the patch are just cosmetic things, changing the interator loops to be range-based for loops. I just made those changes for my own benefit, to make the code easier to read so that I could see what I was doing.

There's only really three lines that make any difference: the sorting of the list, the starting x position when placing an item, and the 'break' rather than 'continue' for quick search (which is just for efficiency).

---
By the way, I like the fix for the psi bug. It looks like you've fixed the caused of the crash, fixed the faulty AI, and improved the robustness of the functions involved.
« Last Edit: July 13, 2017, 01:57:53 am by karadoc »

Offline Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 9091
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1345 on: July 13, 2017, 06:37:46 pm »
Missed bugs are fix.

Thanks, it works now.

I am currently trying to merge oxce 3.9a, but I get compilation errors... can you have a look at the attached picture please?

Offline Yankes

  • Global Moderator
  • Commander
  • ***
  • Posts: 3350
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1346 on: July 13, 2017, 08:02:14 pm »
I see, VS is more restrictive in that case, I will fix it.

[ps]

check if it fix your error: https://github.com/Yankes/OpenXcom/commit/3ce7a4cd72580743a579405f0164df9262e183ec
« Last Edit: July 13, 2017, 08:07:40 pm by Yankes »

Offline Meridian

  • Global Moderator
  • Commander
  • ***
  • Posts: 9091
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1347 on: July 14, 2017, 02:28:21 pm »
I see, VS is more restrictive in that case, I will fix it.
check if it fix your error: https://github.com/Yankes/OpenXcom/commit/3ce7a4cd72580743a579405f0164df9262e183ec

Yes, it's fixed.

I have fixed one more CTD: https://github.com/MeridianOXC/OpenXcom/commit/729f82f07a7cf81bc3e1aac741d1a67dac8fc810
You should cherry-pick that into OXCE too.

And one optional fix/change: https://github.com/MeridianOXC/OpenXcom/commit/327e1ace90ffcf3a4a4cbe2fdea03ac7909519cf
This enables ammo animation also on "combo-ammo weapons" (e.g. throwing knives + grenade laucher)

Offline Yankes

  • Global Moderator
  • Commander
  • ***
  • Posts: 3350
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1348 on: July 14, 2017, 10:50:20 pm »
Yes, it's fixed.

I have fixed one more CTD: https://github.com/MeridianOXC/OpenXcom/commit/729f82f07a7cf81bc3e1aac741d1a67dac8fc810
You should cherry-pick that into OXCE too.

And one optional fix/change: https://github.com/MeridianOXC/OpenXcom/commit/327e1ace90ffcf3a4a4cbe2fdea03ac7909519cf
This enables ammo animation also on "combo-ammo weapons" (e.g. throwing knives + grenade laucher)
Thanks for fixes

Offline kikimoristan

  • Commander
  • *****
  • Posts: 647
    • View Profile
Re: [OXCE] OpenXcom Extended
« Reply #1349 on: July 19, 2017, 03:48:56 pm »
I like that you added up to 20 damage types I was reading the code you got extra D_1 to D_10