Show Posts

This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.

Messages - karadoc

Pages: [1] 2 3 ... 14
OpenXcom Extended / Re: seg-fault bug in TileEngine::addLight
« on: March 21, 2018, 10:28:06 am »
I see. So it's more complex than I thought.

If those coordinates aren't in vexel units, then maybe the problem isn't in that part of the code at all.

The exe I'm using is my own custom version which is very similar to Meridian's build. I've just merged in the latest changes from and I'm still seeing the seg-fault.

Given that source of the crash isn't so clear now; I should mention that I've only noticed this crash on particular maps (landed reticulan ships like the one in the save). So it's possible that there is something wrong with the map that is causing the crash. Ideally the exe would handle a faulty map gracefully without crashing; but nevertheless, it's possible that there is nothing wrong with the logic in the exe after all.

OpenXcom Extended / [Bug?] seg-fault bug in TileEngine::addLight
« on: March 20, 2018, 09:52:22 am »
I'm attaching a save game from xPiratez.
If I load the save and end turn, it often crashes (seg-fault). If it doesn't crash, I just load it again and end turn... until it crashes.

I've tested this using a debug build, and it turns out that the seg-fault is in `TileEngine::addLight`
I don't know much about that function, but I looked into it a bit, and it look like there's something wrong with how the blockVisibility cache is used. I think the code is wrong - but I'm not sure how it is meant to work.

The seg fault is when accessing `cache`, which is a reference to `_blockVisibility[_save->getTileIndex(lastPoint)]`

It turns out the the tile index is out of the bounds of the `_blockVisibility` vector. `_blockVisibility` is allocated the same size as the map. ie. it's size is map width*length*height. In the case of the seg-fault, the height of `lastPoint` is above `_save->getMapSizeZ()`, and so the index is out of bounds - which causes a seg-fault.

The thing is, the calculation for `point` (which later becomes lastPoint) doesn't look like map coordinates. From the way it is calculated, it is clearly possible for it to be out-of-bounds. (Which explains the crash.)

Originally, `point` is supplied as voxel coordinates, and then adjusted as follows:

    point = point / accuracy;

`accuracy` is defined like this:

    const auto divide = (fire ? 8 : 4);
    const auto accuracy = Position(16, 16, 24) / divide;

So you see, `point` is not really map coordinates anyway, and so it isn't surprising that it gets too high for the cache. I'm not sure if the mistake is in how the cache is used, or if it is just that the cache needs to be a lot bigger; but I'd suggest that we shouldn't be using `getTileIndex` for coordinates that have a different scale the the map coordinates, otherwise we'll probably get other bugs regardless of how big the cache is.

In any case, as I said, I'm not sure how it is suppose to work, so that's all I can do to help right now. I hope that's enough info to help someone find and fix the problem.

Melee attacks against units standing on slopes would often fail to do any damage. This was a long standing and well known bug. Today I tracked it down. It turns out to be a mistake in TileEngine::voxelCheck.

The gist of the problem is that the voxel check wasn't finding the unit to deal the damage because it wasn't including the terrain height of the unit. It wasn't including the terrain height of the unit because it was instead including the terrain height of the tile above the unit... which was unhelpful.

The fix is easy, and I've posted it here (along with a description of the problem).

I suspect a side effect of this will be that units on slopes will be hit more often in general; because voxelCheck is used for many things, and this problem would affect all of them. Nevertheless it is a bug fix.

XPiratez / Re: Disruptive transmission mission
« on: July 29, 2017, 03:09:38 am »
I don't think you'll be able to beat it until you have space suits. The enemies there are too dangerous to defeat with the escape pods. You'll want to use proper laser weapons, or perhaps melee.

OpenXcom Extended / Re: [OXCE] OpenXcom Extended
« on: July 12, 2017, 01:51:01 am »
Here is my patch for improving the order of items on the ground.

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.

OpenXcom Extended / [DONE] [Suggestion] Pilot auto assigning ideas
« on: July 11, 2017, 12:10:33 pm »
I still think it's pretty awkward to get thrown out of an intercept action due to lack of pilots. It's not as bad now that there is a button to take you to the ship screen to assign a pilot, that was a big improvement, but it's still a bit cumbersome to have to go through a few menus for something that usually doesn't matter.

I don't think the 'auto assign' option in the options menu is as useful or as intuitive as it could be. Currently the auto assign option forces all ships to use whoever is last on the list as their pilots. This is ok for most situations, but sometimes it just isn't what I want to do - and I certainly don't want to have to go in and out of the options screen whenever I want to change tactics. In general, I care about who pilots my interceptors, but I don't care who pilots the troop transport ships. The tricky part is that some troop transport ships are also interceptors. I think it would be better if auto assign only tried to assign pilots when there aren't enough already.

Here are a few other approaches which I think would work well. (Any one of them would be good. Not all three!)
  • Whenever the player puts any unit in a craft, the game checks if the craft has enough pilots; if it doesn't yet have enough pilots, the unit being added to the craft becomes a pilot. I think this would be good default behaviour, and it would remove the need to even have an option to 'auto assign' pilots.
  • Alternatively, when an intercept is told to launch, if there the ship doesn't have enough pilots, it could then (and only then) automatically assign whoever was available as a pilot. This would be the functionality of the 'auto assign' option. If 'auto assign' was turned off, or if the craft still doesn't have enough pilots even after auto-assigning, the game would do exactly what it does currently. ie. tell the player that there isn't enough pilots and offer to go to the ship screen.
  • There could be an additional button on the 'not enough pilots' screen which effectively says "auto-assign and go!" The button would try to automatically assign pilots, and continue the intercept command without the player having to reissue it.

As I said, I reckon the current system is still a bit awkward. I also think it's potentially confusing for newer players who are less comfortable with the UI. I think it would be much better if one of the suggestions I've offered was implemented. I like the first option best.

OpenXcom Extended / Re: New game setup and pilots
« on: July 11, 2017, 11:41:13 am »
It occurs to me that the stuff that I posted wasn't directly relevant to what this thread is about. The topic of this thread, about having pilots assigned at the start of the new game, is an issue worth discussing; and none of the suggestions I posted would have helped with that. So rather than risking hijacking this thread, I'll move my post to a new thread.

OpenXcom Extended / Re: [OXCE] OpenXcom Extended
« 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.

XPiratez / Re: Bugs & Crash Reports
« on: July 11, 2017, 02:41:25 am »
Shambler hunts pop up without monster hunt beeing researched. Intentional?
I believe so, yes. It has always been that way.

XPiratez / Re: heyyy Freak Out!: PSA about freak gal type.
« on: July 10, 2017, 06:19:24 am »
Makes me think that there's something off in the stat allocation.
I just glanced at the stat allocation code, and it looks like it should work as expected:

Code: [Select]
UnitStats minStats = rules->getMinStats();
UnitStats maxStats = rules->getMaxStats();

_initialStats.tu = RNG::generate(minStats.tu, maxStats.tu);
_initialStats.stamina = RNG::generate(minStats.stamina, maxStats.stamina); = RNG::generate(,;
_initialStats.bravery = RNG::generate(minStats.bravery/10, maxStats.bravery/10)*10;
_initialStats.reactions = RNG::generate(minStats.reactions, maxStats.reactions);
_initialStats.firing = RNG::generate(minStats.firing, maxStats.firing);
_initialStats.throwing = RNG::generate(minStats.throwing, maxStats.throwing);
_initialStats.strength = RNG::generate(minStats.strength, maxStats.strength);
_initialStats.psiStrength = RNG::generate(minStats.psiStrength, maxStats.psiStrength);
_initialStats.melee = RNG::generate(minStats.melee, maxStats.melee);
_initialStats.psiSkill = minStats.psiSkill;

_currentStats = _initialStats;
(Although I don't know why it ignores the max for psi-skill. I would have thought that if the modder didn't want psi-skill above the minimum, then they'd just set the max to be the same as the min. There's no need to make a special hard-coded rule just for that.)

OpenXcom Extended / Re: [OXCE] OpenXcom Extended
« 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.

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.

OpenXcom Extended / Re: [OXCE] OpenXcom Extended
« on: July 09, 2017, 04:05:23 am »
Here's another crash for you:

In the attached save, the game crashes after ending turn due to an 'unsupported action'.

I've looked into it, and found the problem is in `AIModule::psiAction()`
In particular, this part is causing the crash:
Code: [Select]
if (_visibleEnemies)
auto ammo = _attackAction->weapon->getAmmoForAction(_attackAction->type);
if (ammo && ammo->getRules()->getPowerBonus(_attackAction->actor) >= weightToAttack)
return false;
The `getAmmoForAction` crashes in this cause because the _attackAction is BA_RETHINK, which doesn't have associated rules, and hence causes a crash.

This seems easy enough to fix, but the best way to fix it depends a bit on coding style and on what you are actually trying to achieve with this bit of code.

For example, I personally think it's a bit weird that `const RuleItemAction *BattleItem::getActionConf(BattleActionType action) const` throws an exception when the result is NULL. If the result is never allowed to be NULL, then the return type should be a reference, not a pointer. So I'd probably fix the problem by not throwing the exception in `getActionConf`, and instead checking for NULL elsewhere. In this case, if `getActionConf` returns NULL, then `getAmmoForAction` should also return NULL.

If `getActionConf` no longer throws, then it would probably be wise to check for NULL every time it is used. But since no one is trying to catch the exceptions anyway, it won't really make any difference even it does return NULL an no one checked. It will just seg-fault instead of having a uncaught exception.

Anyway, as I said, the best way to fix this crash depends on how you like your code to behave. I'll leave it up to the people in charge. But I will say that in my view, `getAmmoForAction` should not crash no matter what 'action' is given to it. So I suggest that the bug should be fixed in that part rather than in the AI.

On closer inspection, the code already has a function called `getActionConfNullable`, which is the same as `getActionConf` except that it returns null instead of throwing. As I said before, no one is trying to catch these exceptions... so I really don't know why the exception throwing version of the function even exists. In any case, here's a simple patch for the bug. Enjoy.

XPiratez / Re: Bugs & Crash Reports
« on: July 09, 2017, 02:56:33 am »
Disc-O-Death has the same TU cost for aimed and snap shot.
That might be intentional, to ensure that reaction shots have lower accuracy (reaction always uses snap shot).

XPiratez / Re: The most unusual moments at your campaigns
« on: July 07, 2017, 07:39:11 am »
The talk of zombie missions reminded me of a screenshot that I got from early in the campaign.

It was a zombie mission, and I'd cleared it out except for one enemy which I couldn't find. Eventually, "bug hunt mode" activated and so I found where they were...    It turned out to be a chrysalid.

Now, if I'd seen the chrysalid early in the mission I would have just bailed immediately; but this was after I'd already taken ages clearing out the entire map. I didn't want to lose all my loot and waste all that time. So I carefully set up a situation where the 'lid would charge towards me and not quite get a chance to attack, then I'd slam it with a heap of melee attacks; axes, sabres, etc.

The plan worked. At least, the part I described worked. It charged, and I hit it with melee weapons. Unfortunately, it didn't actually die.

The screenshot is what the start of the next turn looked like.

The only reason I had one soldier left alive was that they were already panicking too much to get into position properly for the ambush...

This particular situation went so poorly that I reloaded a geoscape autosave and didn't do the mission. I figured that it was basically game-over if I played on, given that I'd lost my entire crew and my only ship. (There's actually a follow-up story to this as well, but I'll save that for another time.)

XPiratez / Re: [MAIN] XPiratez - 0.99G2 - 4 Jun - Shipwrecked Summer
« on: July 07, 2017, 02:38:10 am »
You know, after spending 500 tokens on it, the electric lasso feels underwhelming. I thought it'd be like cattle prod, but both times I managed to try it out against unarmored human (1 G.O. and 1 Air Sailor), it failed to do it's job, and I hit twice each time. So much for that reliability.

Oh, and there was a word about an enemy base (despite me rocking the chart with 7k+ score). Is it a good idea to camp supply ships, given that I only have turtle, tac armors, early-game firearms and tons of explosives? I could use me some gauss guns.
The key advantage of the electric lasso is its range. It may take a couple of shots, but at least it allows you to it enemies that aren't standing next to you. For example, you can hit them through a window of a building instead of having to walk around to the door.

As for supply ships... if you're the kind of player who reloads when something bad happens, then the supply ships are worth it. If you're the kind of player who accepts bad results as part of the game experience, then you should probably avoid supply ships at this stage. Supply ship missions are very delicate and volatile... things can look like they are going well, and then suddenly you've lost your entire crew.

By the way, you won't be able to use the gauss weapons without extra research anyway.

Pages: [1] 2 3 ... 14