Multi-Craft Hangars, Craft Sizes, Custom Craft Classifications & More...

[WarStalkeR]

I've implemented advanced craft loss algorithm, for cases when hangars damaged or destroyed: https://github.com/WarStalkeR/OpenXcomExMore/compare/a4f0895..ffb2f83?diff=unified

With this update, craft handling on hangar destruction (or ruin) will be handled much better. Basically, if ruined version of facility will have different slots, or some slots will be marked as unusable (they aren't seen in ufopaedia) through use of '-1: STR_CRAFT_CLASS_NO', crafts that can't find new slots at that very instance will be lost (you don't really have time during bombardment to transfer crafts to other slots as matter of the fact). It also works perfectly fine with grouped slots.

Working Hangar Example:

Code: [Select]

  - type: STR_HANGAR_INTERCEPTOR
    craftOptions:
      - [-12, -12, 20, 49, false]
      - [-12, -12, 2, 9, false]
      - [14, -12, 20, 49, false]
      - [14, -12, 2, 9, false]
      - [1, 6, 20, 49, false]
    optionGroups: [2, 2, 1]

Ruined Hangar Example:

Code: [Select]

  - type: STR_HANGAR_INTERCEPTOR_RUIN
    craftOptions:
      - [-12, -12, 20, 49, false]
      - [-12, -12, -1, -1, false]
      - [14, -12, -1, -1, true]
      - [14, -12, -1, -1, true]
      - [1, 6, 20, 49, false]
    optionGroups: [2, 2, 1]

Crafts that were in slots that turned into '-1, -1' slots will be lost. Grouped slots can be used to guarantee survival of vehicle for example, but end up in a loss of bigger craft. Last entry in 'craftClasses:' will be hidden from Ufopaedia (hence use of '-1: STR_CRAFT_CLASS_NO' or something similar is a must), but since Analysis shows craft slots in numbers, they will be visible there.

[psavola]

I've implemented advanced craft loss algorithm, for cases when hangars damaged or destroyed: https://github.com/WarStalkeR/OpenXcomExMore/compare/a4f0895..ffb2f83?diff=unified

With this update, craft handling on hangar destruction (or ruin) will be handled much better. Basically, if ruined version of facility will have different slots, or some slots will be marked as unusable (they aren't seen in ufopaedia) through use of '-1: STR_CRAFT_CLASS_NO', crafts that can't find new slots at that very instance will be lost (you don't really have time during bombardment to transfer crafts to other slots as matter of the fact). It also works perfectly fine with grouped slots.

Out of curiousity, will this work as well or even better than current OXCE with 'missile strikes' (or otherwise hangars getting destroyed - for example a base attack - while the crafts are out of the base)?

Current OXCE behaviour is rather harsh if a hangar gets destroyed while craft(s) are out on a mission out of the base. It seems that essentially it's just RNG which craft gets immediately wiped out. Rather nasty if you lose a transport full of crew, like I did in an ironman run. There could be alternatives for smarter algorithm (with or without UI dialogs what the player wants to do in that case). See: https://openxcom.org/forum/index.php/topic,6668.msg151189.html#msg151189

[WarStalkeR]

Out of curiousity, will this work as well or even better than current OXCE with 'missile strikes' (or otherwise hangars getting destroyed - for example a base attack - while the crafts are out of the base)?

Current OXCE behaviour is rather harsh if a hangar gets destroyed while craft(s) are out on a mission out of the base. It seems that essentially it's just RNG which craft gets immediately wiped out. Rather nasty if you lose a transport full of crew, like I did in an ironman run. There could be alternatives for smarter algorithm (with or without UI dialogs what the player wants to do in that case). See: https://openxcom.org/forum/index.php/topic,6668.msg151189.html#msg151189

You rising a valid point. Hmm... I need to think how to handle it. Especially for edge cases. It also means that if you suddenly end up with more crafts then you have slots, I will need to trigger Sale/Transfer window.

[Alex_D]

My suggestion for extreme cases such as a base loss (not quite the question), is to allow for an immediate re-route of all items and personnel transfers, from the destroyed base to the first base on the list (that remains). It makes sense from a realistic point of view.

I agree, facilities that are destroyed during a bombardment ought to trigger a sales/move event if no enough space remains.

[WarStalkeR]

Well, I've resolved the issue: https://github.com/WarStalkeR/OpenXcomExMore/commit/062ebfd3b5c4d2913496b756368680334e08f089

Now crafts are only destroyed, if after bombardment damaged version of hangar doesn't have suitable slot at exactly same place. If craft is in-flight outside of the base, or damaged hangar has suitable slot at exactly same place for craft - craft isn't lost.

Moreover, if craft is outside and base has no more hangars, you can redirect it to another base with empty hangar (or clear craft from it and then redirect) without any issues. However, if craft returns to base with insufficient hangar slots, you will get pop-up and will be forced to sell or transfer excess crafts.

P. S. Thanks to these changes I've discovered that I have some kind of error in debug spawn missile strike against base. Will be trying to find out what is the reason.

[Yankes]

This handling flying crafts is good solution

[WarStalkeR]

Since my previous pull request with soldiers and vehicles as craft stats got merged, I've started preparations for creating dedicated branch for multi-craft hangars pull request.

Meanwhile, my latest addition (per Yankes guidance) is reworked syntax to make use of new multi-craft hangars feature more bearable.

Now it looks like this:

Code: [Select]

  - type: STR_HANGAR_INTERCEPTOR
    craftsHidden: false
    craftOptions:
      - {x: -12, y: -12, min: 20, max: 49, hide: false}
      - {x: -12, y: -12, min: 2, max: 9, hide: false}
      - {x: 14, y: -12, min: 20, max: 49, hide: false}
      - {x: 14, y: -12, min: 2, max: 9, hide: false}
      - {x: 1, y: 6, min: 20, max: 49, hide: false}
    optionGroups: [2, 2, 1]

Prior update it looked like this:

Code: [Select]

  - type: STR_HANGAR_INTERCEPTOR
    craftsHidden: false
    craftOptions:
      - [-12, -12, 20, 49, false]
      - [-12, -12, 2, 9, false]
      - [14, -12, 20, 49, false]
      - [14, -12, 2, 9, false]
      - [1, 6, 20, 49, false]
    optionGroups: [2, 2, 1]

Related commits code changes can be seen here: https://github.com/WarStalkeR/OpenXcomExMore/compare/0aa946..8b909c?diff=unified

Before creating pull request I'll be double checking that slot grouping feature, which ramps-up code complexity (but not execution time) by order of magnitude, works without any issues.

[Meridian]

I'm sorry, but I never said I will merge your hangar types PR.
I paid a lot of attention to never say that.
I saw a lot of good things in it, some bad too, but I still plan to implement the feature myself.

As I am trying to say from the very beginning to flaubert, to you, and to everyone else in various threads across the forum... please don't waste your time creating a PR for hangar types, I am not going to use it (other than for inspiration).

[WarStalkeR]

I'm sorry, but I never said I will merge your hangar types PR.

Nah, its fine. No need to worry. I haven't forgot what you said. You can use it as reference at very least.

I saw a lot of good things in it, some bad too, but I still plan to implement the feature myself.

Well, all the issues you've raised - I resolved them, including custom craft slot ranges (for mixed slot type hangars), smart craft-to-slot allocation (via heuristic sorting algorithm), proper craft handling for destroyed hangars, understandable syntax & etc.

As I am trying to say from the very beginning to flaubert, to you, and to everyone else in various threads across the forum... please don't waste your time creating a PR for hangar types, I am not going to use it (other than for inspiration).

I'm not wasting time. Even if it won't make into main OXCE code, I will still use it for my submod (well, I'm already using it). You can just take parts of code you find useful (everything is encapsulated into organized functions).

[WarStalkeR]

Meridian, after looking how much fixing and reworking you had to do just for my commit for soldiers and vehicles as craft stats: https://github.com/MeridianOXC/OpenXcom/compare/fcd383b..4139ebb?diff=unified

Now I finally understand, why you told that you want to implement multi-craft hangar feature on your own. Amount of work for fixing/reworking my commit will be so immense that it is just better (and easier) to do everything from zero.

P.S. Now I know that I handled whole code in src/Basescape/CraftWeaponsState.cpp in a wrong manner.

[Meridian]

Now I finally understand, why

Don't let that disappoint you too much.
Small and medium-sized, well-defined and upfront-discussed features are still worth a PR.

PS: thanks for saying you understand why; it was really nice to hear

[WarStalkeR]

Don't let that disappoint you too much.

If anything, I'm only inspired to improve myself.

Small and medium-sized, well-defined and upfront-discussed features are still worth a PR.

When I started multi-craft hangar implementation, I was like "it won't be that big anyway" until I found out how much related files I need to update to support craft slot iterator instead of base facility iterator and standard crafts number value. And it indeed turned into something massive.

PS: thanks for saying you understand why; it was really nice to hear

I mean, after seeing this huge amount of work https://github.com/MeridianOXC/OpenXcom/compare/fcd383b..d6b7fbc?diff=unified you need to put just for this small-sized PR, even oblivious me understands the scale of the disaster my multi-craft hangar code is

And frankly, I'm just happy that there is someone whom I can learn from, how actual C++ code should look like (where I made mistakes, what in code I could've arranged better, where I made logical/algorithmic error & etc).

Because ChatGPT can teach only so much: as of right now it is good at explaining what std::vector<> is, how to use it (sort, search & etc), what are pointers are & etc, but nothing overly complex.

Prior to XPZ and OXCE, I never expected to code in the first place, let alone code in C++ (I hated coding with passion, especially in C++). Nor I ever considered it anywhere near as fun. Yet unexpectedly, when I started implementing first C++ code changes for XPZ submod, it was just as fun and interesting as normal modding itself.

P.S. I still hate idea of coding for anything else beside modding

[WarStalkeR]

Meridian, here is the list of all "end points" (functions, loops & etc) that I had to touch/modify to completely implement my iteration of the feature. I hope this list will make implementation of multi-craft hangars a little bit easier for you.

Savegame/Base.cpp(Base.h) (list of implemented functions for reference):
std::vector<CraftSlot> *getCraftSlots() { return &_craftSlots; } - a way to access variable.
const std::vector<CraftSlot> *getCraftSlots() const { return &_craftSlots; } - constant version of it.
int Base::getCraftSlotCost(...) const - used for sorting crafts.
int Base::getCraftVirtualCost(...) const - virtual version, to include transfers/productions.
void Base::updateCraftSlots() - updates and rearranges slots. When facilities with slots added/removed.
void Base::syncCraftSlots() - updates and rearranges crafts. When crafts are added/removed/modified.
void Base::syncCraftChanges() - just me being lazy. Runs two functions above one after another.
int Base::getFreeCraftSlots(int) const - returns amount of available slots for a craft size (uses virtual).
bool Base::allowCraftRefit(...) const - if craft has suitable slot after refit (size change).

Basescape/BasescapeState.cpp (changes):
BasescapeState::viewMouseWheelUp(...) - added new function to switch setCraftForDrawing() via mouse wheel.
BasescapeState::viewMouseWheelDown(...) - added new function to switch setCraftForDrawing() via mouse wheel.

Basescape/BaseView.cpp (changes):
BaseView::draw() - added craft slot iterator _base->getCraftSlots()->begin() instead of old iterator.
BaseView::draw() - at // Draw crafts used added craft slot iterator instead.
BaseView::draw() - moved fac->setCraftForDrawing() to more suitable place.

Basescape/CraftsState.cpp (changes):
CraftsState::lstCraftsClick(...) - added _base->syncCraftSlots() after // reload the UI comment.

Basescape/CraftWeaponsState.cpp (changes):
CraftWeaponsState::lstWeaponsClick(...) - added _base->syncCraftSlots() prior to _craft->checkup() code.
CraftWeaponsState::lstWeaponsClick(...) - added if (!_base->allowCraftRefit(_craft, newCraftSize)) allowChange = false; to check if there will be new compatible slots due to size change.

Basescape/DismantleFacilityState.cpp (changes):
DismantleFacilityState::btnOkClick(...) - added _base->syncCraftChanges() after _view->setBase(_base) code.
PlaceFacilityState::viewClick(...) - added _base->syncCraftChanges() after _view->setBase(_base) code.

Basescape/ManufactureInfoState.cpp (changes):
ManufactureInfoState::moreUnit(...) - added _base->getFreeCraftSlots(_production->getRules()->getProducedCraft()->getCraftSize()) instead of _base->getAvailableHangars() - _base->getUsedHangars() code (in two instances).

Basescape/ManufactureStartState.cpp (changes):
ManufactureStartState::btnStartClick(...) - added _base->getFreeCraftSlots(_item->getProducedCraft()->getCraftSize()) instead of _base->getAvailableHangars() - _base->getUsedHangars() code.

Basescape/PurchaseState.cpp (changes):
PurchaseState::increaseByValue(...) - added _base->getFreeCraftSlots(ruleC->getCraftSize()) instead of _base->getAvailableHangars() - _base->getUsedHangars() code (two instances).

Basescape/SellState.cpp (changes):
SellState::btnOkClick(...) - added _base->syncCraftSlots() after _base->removeCraft(...) code.
SellState::delayedInit() - added bool overfull = _debriefingState == 0 && Options::storageLimitsEnforced && (_base->storesOverfull() || _base->getUsedHangars() > _base->getAvailableHangars()); instead of bool overfull = _debriefingState == 0 && Options::storageLimitsEnforced && _base->storesOverfull(); code, to handle overflow of crafts due to loss of craft slots.
SellState::delayedInit() - added whole if (_hangarChange != 0) code block with overfull = _debriefingState == 0 && Options::storageLimitsEnforced && (_base->storesOverfull() || (_base->getUsedHangars() + _hangarChange) > _base->getAvailableHangars()), to handle crafts that are 'outside' and not in base.
SellState::updateItemStrings() - added _btnOk->setVisible(!_base->storesOverfull(_spaceChange) && (_base->getUsedHangars() + _hangarChange) <= _base->getAvailableHangars()); instead of _btnOk->setVisible(!_base->storesOverfull(_spaceChange)); code.

Basescape/TransferItemsState.cpp (changes):
TransferItemsState::completeTransfer() - _baseFrom->syncCraftSlots() and _baseTo->syncCraftSlots() in case TRANSFER_CRAFT: before break.
TransferItemsState::increaseByValue(...) - added _baseTo->getFreeCraftSlots(craft->getCraftSize()) instead of _baseTo->getAvailableHangars() - _baseTo->getUsedHangars() code.

Battlescape/DebriefingState.cpp (changes):
DebriefingState::prepareDebriefing() - added _base->syncCraftSlots() after _base->removeCraft(...) code.
DebriefingState::prepareDebriefing() - added base->syncCraftChanges() after base->destroyDisconnectedFacilities() code.

Geoscape/ConfirmDestinationState.cpp (changes):
ConfirmDestinationState::btnTransferClick(...) - added currentBase->syncCraftSlots() and targetBase->syncCraftSlots() at the end of // Transfer craft code block (before _game->popState() code).
ConfirmDestinationState::btnTransferClick(...) - added targetBase->getFreeCraftSlots(_crafts.front()->getCraftSize()) instead of targetBase->getAvailableHangars() - targetBase->getUsedHangars() code.

Geoscape/GeoscapeState.cpp (changes):
GeoscapeState::time5Seconds() - added xbase->syncCraftSlots() after xbase->removeCraft(...) code in // craft destroyed in dogfight code block.
GeoscapeState::handle(...) - added xbase->syncCraftChanges() in the end of for (auto* facility : *xbase->getFacilities()) code block.
GeoscapeState::time1Day() - added if (facility->getRules()->getCrafts() > 0) xbase->syncCraftChanges() in the end of if (facility->getBuildTime() == 0) code block.
GeoscapeState::time1Hour() - added if ((xbase->getUsedHangars() - numCraftsOut) > xbase->getAvailableHangars()) code block at the end of for (auto* xbase : *_game->getSavedGame()->getBases()) loop to trigger forced sale/transfer of crafts, if there are not enough craft slots for crafts that already in base (not flying outside).
GeoscapeState::handleBaseDefense(...) - added if (base->getUsedHangars() > base->getAvailableHangars()) that shows notification message (and does nothing else), if you due to missile strike you've lost hangars and outside crafts don't have enough craft slots in base.

Menu/NewBattleState.cpp (changes):
NewBattleState::load(...) - added base->syncCraftSlots() at the end of if (base->getCrafts()->empty()) if code block.
NewBattleState::initSave() - added base->syncCraftSlots() after base->getCrafts()->push_back(_craft) and prior to // Generate soldiers code block.

Savegame/Base.cpp (changes):
Base::load(...) - added syncCraftChanges() prior to isOverlappingOrOverflowing() code at the end of the function.
Base::damageFacilities(...) - added syncCraftChanges() at the very end of the function, after if (!_mod->getDestroyedFacility()) code block.
Base::damageFacility(...) - added for (auto& craftSlot : _craftSlots) to check what craft slots are preserved and what are lost.
Base::destroyFacility(...) - added for (auto& craftSlot : _craftSlots) to check what crafts are lost in slots that weren't preserved.

Savegame/Production.cpp (changes):
productionProgress_e Production::step(...) - added b->syncCraftSlots() in the end of if (ruleCraft) code block.
Production::startItem(...) const - added b->syncCraftSlots() in the if (c->getRules() == i.first) code block, after removeCraft() code.

Savegame/SaveConverter.cpp (changes):
SaveConverter::loadDatCraft() - added b->syncCraftSlots() in the end of if (base != 0xFFFF) code block.
SaveConverter::loadDatBase() - added if (days == 0 && facility->getRules()->getCrafts() > 0) base->syncCraftChanges() in the end of if (facilityType < _rules->getFacilities().size()) code block.

Savegame/Transfer.cpp (changes):
Transfer::advance(...) - added base->syncCraftSlots() in the end of else if (_craft != 0) code block.

[ontherun]

[...]
Well, while making submod for XPZ I already drawn various hangars... And I already partially made 3x3 hangars.

[...]

i really like this, plus those hangars are very cool! Hope. In the meantime it iis possible yo download that submod?

[Finnik]

If anything, I'm only inspired to improve myself.
When I started multi-craft hangar implementation, I was like "it won't be that big anyway" until I found out how much related files I need to update to support craft slot iterator instead of base facility iterator and standard crafts number value. And it indeed turned into something massive.

After taking some time to develop my branch, I have to say, that every feature I want to implement will take 3-5 more time to be finished, than I estimate =)