OpenXcom Forum
OpenXcom Forks => OXCE Suggestions NEW => OpenXcom Extended (OXCE) => OXCE Suggestions OK => Topic started by: WarStalkeR on September 21, 2023, 04:22:57 pm
-
Hello Meridian, I spoke with Yankes in Matrix Chat and he told me that you intend to implement Multi-Craft Hangars. I want to know if you intended to implement Multi-Craft Hangars in a way, similar to what I implemented here: https://github.com/WarStalkeR/OpenXcomExMore/compare/810aba6..8e4cb43?diff=unified
Sadly, this compare is quite messy as it encompasses multiple code changes, including Multi-Craft Hangars, Craft Sizes, Custom/Moddable Craft Classifications, Bigger Craft Sprites and even soldiers+vehicles as modifiable craft stats (that already in pull into main and QA'd by Yankes).
I'm mostly active on Matrix and all glaring mistakes that were pointed out by Yankes - are fixed, except few: missing proper Struct for hangar slots instead of Position and missing proper Struct for bigOffset. Because I make it all for submod for XPZ, it uses OXCE 7.9.8 as base version. Latest compiled EXE is also uploaded there (in Submods chatroom).
Meridian, you don't need to do any QA, just tell me if this is same or similar way you intended to implement? And I will work from there.
And this is how it looks for modders right now (more description is on GitHub):facilities:
- type: NEW_FANCY_HANGAR
crafts: 2 #Original code isn't going anywhere for sake of max backwards compatibility.
craftsHidden: false #Flag to render or not render housed crafts in base view. Mostly for damaged/sealed hangars.
craftOptions: #Horizontal offset - X, Vertical offset - Y, Craft size limit Z (0 is no size limit and default).
- [2, -4, 37] #This slot can fit size 37 craft or smaller.
- [8, 2, -25] #This slot can fit size 24 craft or smaller and is always hidden, regardless of the 'craftsHidden'The - [2, -4, 37] will be replaced with - {x: 2, y: -4, size: 37, hidden: false} per Yankes advice. I originally intended to go with - [2, -4, 37, false] but Yankes said its better to make it more verbose and obvious, which I agree with.
crafts:
- type: NEW_FANCY_CRAFT
craftSize: 12 #Can't be placed into size 11 hangar slots. If 0, can be placed into any hangar slot (0 is default). The craftSize variable has no bounds beside being an int (i.e. from -2^31 to 2^31). Modders can define for themselves what craft size ranges belongs to what craft classes.craftWeapons:
- type: NEW_FANCY_WEAPON
stats:
craftSize: 6 #Increases craft's size by 6, if equipped. Can't be equipped if no suitable hangar slots available.craftSize is stat as so modders can create various types that take more or less space on craft.craftClasses:
100: STR_CRAFT_CLASS_03 #All crafts of size 100 or above will be assigned STR_CRAFT_CLASS_03 string.
20: STR_CRAFT_CLASS_02 #All crafts of size 20 to 99 will be assigned STR_CRAFT_CLASS_02 string.
1: STR_CRAFT_CLASS_01 #All crafts of size 1 to 19 will be assigned STR_CRAFT_CLASS_01 string.
0: STR_CRAFT_CLASS_NA #All crafts of size 0 or below will be assigned STR_CRAFT_CLASS_NA string.
craftsCanChangeClass: false #Allows to enforce craft size changes to be within boundaries declared in the craftClasses.This is global option and can be added just to any rule file. craftsCanChangeClass will allow modders to ensure that player can't install quad 30mm autocannon that increases craftSize by 20 (just because it is in light weapons group) on small helicopter.crafts:
- type: BIG_SPRITE_CRAFT
bigOffsetX: -11 #Because sprite width got increased by 22, i.e. from 32 to 54.
bigOffsetY: -16 #Because sprite height got increased by 32, i.e. from 40 to 72.Allows to keep crafts with bigger sprites centered in hangars at same point as original 32x40 sprites. Any change to the sprite size should be even: i.e. you can't use 35x45 pixels sprite. Any change to sprite size should get reflected as negative half in bigOffsetX and bigOffsetY. Rather lazy implementation, but I will replace it with custom Struct as well.
-
Hello Meridian, I spoke with Yankes in Matrix Chat and he told me that you intend to implement Multi-Craft Hangars. I want to know if you intended to implement Multi-Craft Hangars in a way, similar to what I implemented here: https://github.com/WarStalkeR/OpenXcomExMore/compare/810aba6..8e4cb43?diff=unified
Sadly, this compare is quite messy as it encompasses multiple code changes, including Multi-Craft Hangars, Craft Sizes, Custom/Moddable Craft Classifications, Bigger Craft Sprites and even soldiers+vehicles as modifiable craft stats (that already in pull into main and QA'd by Yankes).
I'm mostly active on Matrix and all glaring mistakes that were pointed out by Yankes - are fixed, except few: missing proper Struct for hangar slots instead of Position and missing proper Struct for bigOffset. Because I make it all for submod for XPZ, it uses OXCE 7.9.8 as base version. Latest compiled EXE is also uploaded there (in Submods chatroom).
Meridian, you don't need to do any QA, just tell me if this is same or similar way you intended to implement? And I will work from there.
And this is how it looks for modders right now (more description is on GitHub):facilities:
- type: NEW_FANCY_HANGAR
crafts: 2 #Original code isn't going anywhere for sake of max backwards compatibility.
craftsHidden: false #Flag to render or not render housed crafts in base view. Mostly for damaged/sealed hangars.
craftOptions: #Horizontal offset - X, Vertical offset - Y, Craft size limit Z (0 is no size limit and default).
- [2, -4, 37] #This slot can fit size 37 craft or smaller.
- [8, 2, -25] #This slot can fit size 24 craft or smaller and is always hidden, regardless of the 'craftsHidden'The - [2, -4, 37] will be replaced with - {x: 2, y: -4, size: 37, hidden: false} per Yankes advice. I originally intended to go with - [2, -4, 37, false] but Yankes said its better to make it more verbose and obvious, which I agree with.
crafts:
- type: NEW_FANCY_CRAFT
craftSize: 12 #Can't be placed into size 11 hangar slots. If 0, can be placed into any hangar slot (0 is default). The craftSize variable has no bounds beside being an int (i.e. from -2^31 to 2^31). Modders can define for themselves what craft size ranges belongs to what craft classes.craftWeapons:
- type: NEW_FANCY_WEAPON
stats:
craftSize: 6 #Increases craft's size by 6, if equipped. Can't be equipped if no suitable hangar slots available.craftSize is stat as so modders can create various types that take more or less space on craft.craftClasses:
100: STR_CRAFT_CLASS_03 #All crafts of size 100 or above will be assigned STR_CRAFT_CLASS_03 string.
20: STR_CRAFT_CLASS_02 #All crafts of size 20 to 99 will be assigned STR_CRAFT_CLASS_02 string.
1: STR_CRAFT_CLASS_01 #All crafts of size 1 to 19 will be assigned STR_CRAFT_CLASS_01 string.
0: STR_CRAFT_CLASS_NA #All crafts of size 0 or below will be assigned STR_CRAFT_CLASS_NA string.
craftsCanChangeClass: false #Allows to enforce craft size changes to be within boundaries declared in the craftClasses.This is global option and can be added just to any rule file. craftsCanChangeClass will allow modders to ensure that player can't install quad 30mm autocannon that increases craftSize by 20 (just because it is in light weapons group) on small helicopter.crafts:
- type: BIG_SPRITE_CRAFT
bigOffsetX: -11 #Because sprite width got increased by 22, i.e. from 32 to 54.
bigOffsetY: -16 #Because sprite height got increased by 32, i.e. from 40 to 72.Allows to keep crafts with bigger sprites centered in hangars at same point as original 32x40 sprites. Any change to the sprite size should be even: i.e. you can't use 35x45 pixels sprite. Any change to sprite size should get reflected as negative half in bigOffsetX and bigOffsetY. Rather lazy implementation, but I will replace it with custom Struct as well.
Your solution is very similar to mine:
https://r.tapatalk.com/shareLink/topic?url=https%3A%2F%2Fopenxcom%2Eorg%2Fforum%2Findex%2Ephp%3Ftopic%3D10870%2E0&share_tid=10870&share_fid=91863&share_type=t&link_source=app
But in mine, crafts can only be allocated in an specific hangar class. I thought about doing size-basis-allocation, but someone told me that couldn't be enough as you can have small air-craft and small ships and each one could have its own place.
I tried with a bit-wise allocation to allow many hangars allocation for a craft, but It was very difficult to achieve due to algorithmical complexity.
Also, I don't understand the hidden flag, as I thought damaged hangars couldn't store anything; perhaps I'm mistaken.
You should add something to allow current mods compatibility -without changes-. In my version, as I identify hangars/crafts by a number, I used a special-default value (-1) for not-defined hangars, so It was compatible with existing mods.
Furthermore, last version of OXCE+ I worked with, had some logic faults in program that didn't allow many crafts in an hangar. In that version, when a hangar is destroyed, only one craft -the one shown at basescape- is destroyed. So, if you want to show more than one craft at an hangar, that Code must be changed -with I did already in my version-
Hope your version will be accepted. Good luck.
Enviado desde mi V2244 mediante Tapatalk
-
But in mine, crafts can only be allocated in an specific hangar class. I thought about doing size-basis-allocation, but someone told me that couldn't be enough as you can have small air-craft and small ships and each one could have its own place.
I will just add 'minSize', since I will be making my own struct anyway and I intended to add it in the first place.
I tried with a bit-wise allocation to allow many hangars allocation for a craft, but It was very difficult to achieve due to algorithmical complexity.
It isn't simple, but you can use as reference how 'Base.cpp' handles '_crafts' - and if you will handle '_craftSlots' in similar manner, everything becomes way less complicated.
Also, I don't understand the hidden flag, as I thought damaged hangars couldn't store anything; perhaps I'm mistaken.
Example: hangar with blast doors. After bombardment doors closed, hangar damaged, but not heavily. I don't want to draw crafts on top of blast doors, nor do I want to move them to another slot.
You should add something to allow current mods compatibility -without changes-. In my version, as I identify hangars/crafts by a number, I used a special-default value (-1) for not-defined hangars, so It was compatible with existing mods.
I only need to ensure backwards compatibility with OXCE, so in case if original OXCE executable is getting used, so mod will load without any issues.
Furthermore, last version of OXCE+ I worked with, had some logic faults in program that didn't allow many crafts in an hangar. In that version, when a hangar is destroyed, only one craft -the one shown at basescape- is destroyed.
Seems like I forgot to test and validate that part. Thanks for pointing it out.
-
Hi WarStalkeR,
I intend to keep facilities.crafts, same as you.
Your facilities.craftsHidden is a nice idea, I will add it to my notes.
Your facilities.craftOptions I plan to do as well, maybe in a different way, but same result.
craft.craftSize, I don't plan doing:
- I plan a solution based on matching hangar/craft types, similar approach as Flaubert
- I don't think a numeric size should be the core parameter, I'd rather see descriptive types (e.g. small/medium/large, land/underwater, earth/space, etc.); I think it's better for players and modders
craftWeapons.craftSize, I don't plan doing either:
- honestly any weapon should be much smaller than a craft and not change its size
(- or from the other perspective... craft size is the size of the craft with the biggest possible guns already installed)
craftClasses, I don't plan on doing as global parameters, but as hangar and craft types directly.
--
I also will make sure the solution is backwards compatible with current mods, with previous versions of OXCE and with previous and future versions of OXC. Any old saves from OXC/OXCE should load and recover in the new version of OXCE, and any new OXCE saves should load and recover in the old versions of OXC/OXCE.
--
Functionally, I plan to change existing OXC (vanilla) behavior only where absolutely necessary for data consistency... for example as Flaubert mentioned when hangars are destroyed.
--
I'm not sure if your size-based solution will be compatible (or necessary) after the planned OXCE changes are done.
I guess we'll see only after it's done.
-
craft.craftSize, I don't plan doing:
- I plan a solution based on matching hangar/craft types, similar approach as Flaubert
- I don't think a numeric size should be the core parameter, I'd rather see descriptive types (e.g. small/medium/large, land/underwater, earth/space, etc.); I think it's better for players and modders
craftWeapons.craftSize, I don't plan doing either:
- honestly any weapon should be much smaller than a craft and not change its size
(- or from the other perspective... craft size is the size of the craft with the biggest possible guns already installed)
craftClasses, I don't plan on doing as global parameters, but as hangar and craft types directly.
My approach to the Craft Size stems from this example:
Escort-class, Craft Size range: 20 ~ 29.
Small-class, Craft Size range: 30 ~ 49.
"Piranha" belongs to Escort-class, while "Shark" belongs to Small-class.
Both of them have same weapon slots: 2x Light Weapons.
Research, "Minigun Development", unlocks set of new Light Weapons:
* 14mm Minigun - increases craft's size by 4.
* 14mm Dual Minigun w/ Extended Ammo Rack - increases craft size by 8.
* 14mm Minigun w/ Advanced Cooling - increases craft size by 7.
* 14mm Minigun w/ Reduced Ammo Cap - increases craft size by 3.
Piranha won't be able to mount two biggest Minigun variants, because it ain't big itself.
Shark will be able to mount two biggest Minigun variants, because it has enough space to spare for it.
Another example will be module/system that increase craft's soldier/vehicle (the part that transforms soldiers/vehicles to dynamic stat is already in pull and QA'd by Yankes). It should be rather massive, to ensure that only one can be mounted into same system slots that allows to mount various other systems and to make player chose between better soldier capacity or better craft weapons.
Craft Size is something akin to inventory slots, albeit numerical only.
-
I understand.
I just don't think it's a good feature.
Both usability-wise and lore-wise.
In my opinion, a jet without a minigun and a jet with a minigun makes no difference.
Hangar space/volume that can house a jet without a minigun must be able to house a jet with a minigun, easily.
First because a minigun is way smaller than any jet, and second because any hangar slot must have a generous "buffer" (crafts in a hangar are not tetris pieces literally touching each other, there's always a LOT of extra space around a craft for maneuvering, refueling, repairs, rearming, installation of weapons, takeoff and landing, etc.).
Volume-wise, I don't think a craft takes more than 10% of the volume of its assigned slot, probably even less. With a minigun installed, it takes up 10.1% of the available volume.
As for "extra modules" increasing craft crew capacity... I have already said that I don't have any objections and that the PR can be merged into OXCE.
But I also hope that modders have common sense and won't allow adding a "weapon" to a single-manned jet that will increase its capacity by another 10 people or something.
I would (maybe naively) assume that adding a little extra capacity to a small craft leaves it still small, medium stays medium and large stays large.
PS: I definitely don't want hangars to work like soldier inventory... that is way too complicated and unnecessary.
PS2: if you're on Matrix, maybe ask Dioxine (and other modders) what do they think about the concept... if they want to go to this level of granularity... or if they would prefer "simpler" categorization (by type instead of by numbers). So far, all the requests I have heard over the years from various people were just fine with the types.
-
In my opinion, a jet without a minigun and a jet with a minigun makes no difference.
Hangar space/volume that can house a jet without a minigun must be able to house a jet with a minigun, easily.
First because a minigun is way smaller than any jet, and second because any hangar slot must have a generous "buffer"
When I started implementing craft slots at first I went similar to the way you intended. Craft gets size/id/type as single number and then in slots I write what sizes/ids/types it can house. But then I though maybe it will be good idea to submod different variations of existing weapons/modules/system in XPZ and allow players to balance it out on their own: do the player wants craft weapon with more ammo, but gets to equip weaker module/system? Or do the player wants strap onto craft additional rocket engines in exchange for meager weapons with reduced stats. And this is how it ended up with craft size becoming new stat for crafts.
(crafts in a hangar are not tetris pieces literally touching each other, there's always a LOT of extra space around a craft for maneuvering, refueling, repairs, rearming, installation of weapons, takeoff and landing, etc.).
Well, while making submod for XPZ I already drawn various hangars... And I already partially made 3x3 hangars.
(https://i.imgur.com/GH7J0zu.png)
* Old Civilian Hangar - can safekeep and maintain single Assault-class craft. Also known as default hangar.
* Reinforced Hangar - can safekeep and maintain single Massive-class craft.
* Composite Hangar - can safekeep and maintain one Large-class and one Medium-class craft.
* Interception Hangar - can safekeep and maintain three Small-class crafts.
* Relic X-COM Hangar - can safekeep and maintain one Massive-class and two Escort-class crafts. Has blast doors.
* Camping Site - can house single Infantry-class "craft" (expeditions & etc).
* Vehicle Garage - can house any single Vehicle-class "craft" (truck, convoy, cadillac & etc).
* Small Helipad - can house any single Escort-class craft (small helicopter, Piranha & etc).
* Mass Driver - can house four Rail Pods (they will get new sprite for it as well).
PS: I definitely don't want hangars to work like soldier inventory... that is way too complicated and unnecessary.
Well, Craft Size is numerical only to avoid all this hassle (despite my love for X-COM Apocalypse-style craft inventory, implementation of which into OXCE is just beyond my ability). It is more like mix of inventory and max weight capacity. Anyway, any code you find yourself in my fork, you're free to take and modify as you deem necessary.
PS2: if you're on Matrix, maybe ask Dioxine (and other modders) what do they think about the concept... if they want to go to this level of granularity... or if they would prefer "simpler" categorization (by type instead of by numbers). So far, all the requests I have heard over the years from various people were just fine with the types.
Certainly. I will ask Dioxine and other modders what they are thinking in that regard. Frankly, I hope the code I've wrote already will be of help to you and reduce some of your OXCE workload and allow you to implement stuff you want instead.
-
I consider this system far overcomplicated for my actual needs (for which a system mirroring eg. prison types would be more than enough; I actually plan to have only 2 types of craft, 'small' and 'normal', to make craft like V8 more useful by making it take 1/4th space). However I won't judge or try to guess what other people need.
-
Coincidentally, my opinion is exactly the same. I don't even understand 90% of these features, all I want for my mod are two types of hangars, big and small. Right now I honestly don't know what else I could ever want and why.
But then again, I don't like the air game in X-Com much, so I'm not the best person to ask.
-
Coincidentally, my opinion is exactly the same. I don't even understand 90% of these features, all I want for my mod are two types of hangars, big and small. Right now I honestly don't know what else I could ever want and why.
But then again, I don't like the air game in X-Com much, so I'm not the best person to ask.
Honestly, most people only really wanted a 1x1garage for cars, so first version I had was just "Small&Big hangars/crafts" - where cars (1x1) could be stored also at crafts hangars (2x2) but not the opposite-
Then someone noted that you could need different hangars for underwater and aerial crafts and so on, so a new version was done where each craft had a hangar-ID and could only be allocated at that hangar/garage or whatever. This gives more flexibility, as different crafts can go to the same hangar type, but on the other side they can go to only ONE hangar type, which is the main drawback.
Probably Meridian wants to get ahead of other future modders, whose requirements could be more demanding.
-
I want multiple craft types into multiple hangar types (or more precisely into multiple hangar slot types).
So all the cases you mentioned, plus more.
That should cover all the use cases I have heard so far from the modders... until WarStalkeR came with a more demanding requirement.
I don't dare guess or judge peoples' needs, but I can't make everybody happy.
I will implement the solution acceptable by the majority, and if WarStalkeR will still need a more complicated solution, we can discuss if it could be done in OXCE (afterwards) or if a new fork is required.
-
For what is worth, here is my two cents on my understanding of the hangar problem.
The idea was to approach it like in real life, like storing stuff in a warehouse (or even a large US supercarrier), this is using available square footage area.
A global variable is defined which is the square area per craft size. Example for an array with five class sizes: [100,5,2,10,500], where craft size 1 is 100 area (default), size 2 is 5 area, size 3 is 2 area, and so on. Default array would be [100] in this example.
Each applicable facility will have a number than indicate a total footage area (say 100 default) that can be used to store vehicles, and an array of 1 or 0 indicating if a certain craft size will be allowed to "park" in the facility or not (e.g. [1,0,0,1,0] ). Alternative this array can be used to limit the number of crafts per size class, overruling area limitations (e.g. [1,0,0,3,0]).
Each craft will be assigned a size based on the array position (eg. size=1).
Then then game, once every in a while (once a day, or hour, or when there is a need for a check), will allocate the crafts sorting them across all facilities per base.
Of course this approaches doesn't solve graphics or user specific assignments.
-
I will implement the solution acceptable by the majority, and if WarStalkeR will still need a more complicated solution, we can discuss if it could be done in OXCE (afterwards) or if a new fork is required.
I mean, I already implemented it in my fork. With all the validation to ensure that no action allows to exploit/bypass it. Such as when player purchases/produces/transfers craft, the game validates that base has suitable craft slots, and so that crafts won't occupy bigger slot than they actually need, and some validations for some other craft slot related changes such as when facility gets removed/destroyed/built, and so on.
I've tried to to encapsulate my new code to functions as much as possible (and I think I succeeded for the most part), so just in case anybody can implement their own vision of the multi-craft hangars without worrying too much, if X/Y/Z places were validated properly.
Anyway, Meridian, you don't need to worry about my requests, you can go on with what majority decided. Once XPZ will switch to the new version, I will just sync and rebase my code to these changes (of version XPZ will be using). The C++ code of OXCE (that I've seen so far) is very clean and easy to understand, so there will be no issues with code rebases for me.
-
What I like about your solution is that it has a very low complexity, basically just n.log(n)
You can easily find the best solution for craft placement, and quickly.
That's the advantage of having just one "dimension", i.e. size, and even a numeric one.
However, I (or better said modders) need more than just size, there's demand for orthogonal dimensions (like land/underwater/space, etc.) making craft types inherently incomparable/un-sortable.
This will leave me with a brute-force solution only, resulting in unacceptable complexity of n-factorial.
The workaround I have in mind for that, is basically dumb down the algorithm to quadratic (or even linear) approximation and let the player resolve any failures (situations when the algorithm cannot find a solution) manually.
Your "encapsulation" looks nice, I will give it a try when doing my solution.
I cannot say upfront if it will work for me or not, but I can at least try.
-
However, I (or better said modders) need more than just size, there's demand for orthogonal dimensions (like land/underwater/space, etc.) making craft types inherently incomparable/un-sortable.
I have very good news regarding this part.
I just finished implementing (and testing) min/max size option for hangar slots: https://github.com/WarStalkeR/OpenXcomExMore/commit/07a9cd7bcaa81f3773b8cdcc74c3c3b9bba2e54c
Basically, right now modder can decide that craft sizes from 200 to 399 are reserved for subs only, 400 to 599 to spaceships only & etc. And modding-wise it looks like this:
facilities:
- type: NEW_FANCY_HANGAR
crafts: 3 #Original code isn't going anywhere for sake of max backwards compatibility.
craftsHidden: false #Flag to render or not render housed crafts in base view. Mostly for damaged/sealed hangars.
craftOptions: #Horizontal offset - x, Vertical offset - y, Craft minimum size - min, Craft maximum size - max, Hide craft - true/false.
- [2, -4, 20, 49, false] #This slot can fit any craft of size between 20 (inclusive) and 49 (inclusive), and craft isn't hidden.
- [8, 2, 1, 19, true] #This slot can fit any craft of size between 1 and 19, and always hides craft, regardless of the 'craftsHidden' setting.
- [2, -4, 0, 0, false] #This slot can fit any craft of any size, because 0, 0 is option to ignore craft sizes and classic hangars default.
Using granular feature I've shown in first post modder can classify it like this:
craftClasses:
400: STR_STOP #To limit spaceship class to craft size range between 200 and 399.
200: STR_CRAFT_CLASS_SPACE #All crafts of size 200 to 399 will be reserved for spaceships.
1: STR_CRAFT_CLASS_SUBS #All crafts of size 1 to 199 will be reserved for subs.
0: STR_NOPE #No using 0, since craft size 0 means craft can be shoved into any craft slot.
Or if modder decided to go more complicated way:
craftClasses:
4000: STR_STOP #To limit large spaceship class to max craft size of 3999.
1000: STR_SPACESHIP_LARGE #Craft sizes from 1000 to 3999 - large spaceships.
500: STR_SPACESHIP_MED #Craft sizes from 500 to 999 - medium spaceships.
200: STR_SPACESHIP_SMALL #Craft sizes from 200 to 499 - small spaceships.
100: STR_SUBS_LARGE #Craft sizes from 100 to 199 will be reserved for large subs.
40: STR_SUBS_MED #Craft sizes from 40 to 99 will be reserved for medium subs.
1: STR_SUBS_SMALL #Craft sizes from 1 to 39 will be reserved for small subs.
0: STR_NOPE #No using 0, since craft size 0 means craft can be shoved into any craft slot.
And then just use these self-defined bounds in custom spaceship dock:
facilities:
- type: DEFINITELY_A_SPACEPORT
crafts: 2
craftOptions:
- [10, -10, 1000, 3999, false] #Can house large spaceship.
- [10, -10, 200, 999, false] #Can house small or medium spaceship.
Class also will be shown in Ufopaedia, as in example below:
(https://i.imgur.com/G9BPDwy.png)
-
Very clever way of sneaking in a second dimension, and even without increasing (computational) complexity by much, I have to give you that.
And it MIGHT just be enough, I honestly don't know, I'll need to think about it more and weigh all the cons and pros.
Pros (same as before):
- much better computational complexity than my solution
- guaranteed solution (if it exists)
- no manual craft allocation (although having manual craft allocation isn't necessarily a con, it could be useful as a parallel feature for all of us "OCD" guys)
Cons:
1. not the most intuitive syntax for modders
2. mental gymnastics needed to design the number ranges for craft classes
3. easy to fail the mental gymnastics, e.g. too small ranges, resulting in overflow between ranges caused by added weight/size (from weapons)
4. only 2 dimensions... yes, I know, with even more mental gymnastics, you could encode any number of dimensions into natural numbers, but that's honestly too much to ask for
5. inability to define arbitrary/custom combinations, e.g. if land=1-99, water=100-199, space=200-299, I can't make a hangar for land+space crafts
Cons severity (for me):
Low: 1
Mildly annoying: 2, 3
Questionable: 4
Possible deal-breaker: 5
Unless I'm mistaken, point 5 is where flexibility is lost, but computational complexity is capped at pleasant levels.
Not an easy choice.
-
1. not the most intuitive syntax for modders
It always can be altered. You can decide on better syntax. It's just I got used to '- [A, B, C, D, E]' so much that went along with it. Yankes suggested to go with '{x:, y:, min:, max, hide:}' - but I didn't had enough time to analyze similar code in order to implement it.
2. mental gymnastics needed to design the number ranges for craft classes
Well, this code is pretty much carbon copy from 'monthlyRatings' and 'missionRatings' - I didn't want to make something even more complicated and I couldn't see any simpler solution.
3. easy to fail the mental gymnastics, e.g. too small ranges, resulting in overflow between ranges caused by added weight/size (from weapons)
That's only if modder decides to go along with weapons/systems increasing craft size - then it is unavoidable deep dive. If modder decided to stick to the craft size only for craft classification and hangar slots, then it isn't complicated. Also, overflow won't happen as long as default 'craftsCanChangeClass: false' isn't changed.
4. only 2 dimensions... yes, I know, with even more mental gymnastics, you could encode any number of dimensions into natural numbers, but that's honestly too much to ask for
The more dimensions there are, the more computational complexity increases. I already was horrified how complicated 'Base::getFreeCraftSlots(craftSize)' became. If before I was easily handling everything via 'int freeSlotsNum', with min/max I had to switch to 'std::vector<std::pair<int, int>> freeSlots'.
5. inability to define arbitrary/custom combinations, e.g. if land=1-99, water=100-199, space=200-299, I can't make a hangar for land+space crafts
Yeah, in the same slot you won't be able to put ranges 1-99 and 200-299. You will need to define two different slots for it. On the other side, I can add another facility variable `craftCap` (or 'craftOptionGroups') that will allow you to use only one of two slots - it won't be even that much complicated, compared to refactor I already did in order to implement min/max.
EDIT: Now that I think... The 'craftOptionGroups' might be even simpler solution than I thought and it will allow you to achieve the effect you want and almost without increasing computation complexity.
-
It always can be altered. You can decide on better syntax. It's just I got used to '- [A, B, C, D, E]' so much that went along with it. Yankes suggested to go with '{x:, y:, min:, max, hide:}' - but I didn't had enough time to analyze similar code in order to implement it.
Well, this code is pretty much carbon copy from 'monthlyRatings' and 'missionRatings' - I didn't want to make something even more complicated and I couldn't see any simpler solution.
That's only if modder decides to go along with weapons/systems increasing craft size - then it is unavoidable deep dive. If modder decided to stick to the craft size only for craft classification and hangar slots, then it isn't complicated. Also, overflow won't happen as long as default 'craftsCanChangeClass: false' isn't changed.
The more dimensions there are, the more computational complexity increases. I already was horrified how complicated 'Base::getFreeCraftSlots(craftSize)' became. If before I was easily handling everything via 'int freeSlotsNum', with min/max I had to switch to 'std::vector<std::pair<int, int>> freeSlots'.
Yeah, in the same slot you won't be able to put ranges 1-99 and 200-299. You will need to define two different slots for it. On the other side, I can add another facility variable `craftCap` (or 'craftOptionGroups') that will allow you to use only one of two slots - it won't be even that much complicated, compared to refactor I already did in order to implement min/max.
EDIT: Now that I think... The 'craftOptionGroups' might be even simpler solution than I thought and it will allow you to achieve the effect you want and almost without increasing computation complexity.
Why don't you use bit groups for every sub-decision category? Computationally IS not much burden, as bit masks/bit shifts are computationally easy to manage.
This way every craft/hangar would be made of a "single number" which is actually composed of it's several subgroups classification. One-on-one assignation would be on that number basis. And one-to-many on comparing just the meaningful bits.
My only concern with this approach is about allocation management when you buy/transfer/produce crafts but, with a "disciplined"code, sure It can be achieved.
(Of course, I'm talking about internal representation/management; on the modder side they would just need a enumerated list of characteristics as you proposed; conversion would be done when reading ruleset and just for i ternal rep/management)
-
Maybe I understand something incorrectly, or maybe I'm simply wrong, but:
1/ we don't have a problem with space constraints, there is nothing forcing us to use bit maps, we can easily use numbers or even strings... and all 3 internal representations will have similar complexity (on the same order of magnitude)
2/ I assume your "single number" representation would allow all value combinations in all dimensions... in which case WarStalkeR's quick allocation algorithm wouldn't work, his algorithm poses constraints on the allowed combinations in order to not have to brute-force
(or if your representation poses same constraints, then I don't see the advantage over the solution he already implemented)
-
Why don't you use bit groups for every sub-decision category? Computationally IS not much burden, as bit masks/bit shifts are computationally easy to manage.
Why complicate things, when 'std::vector<>' handling in C++ already does everything for you?
Maybe I understand something incorrectly, or maybe I'm simply wrong, but:
1/ we don't have a problem with space constraints, there is nothing forcing us to use bit maps, we can easily use numbers or even strings... and all 3 internal representations will have similar complexity (on the same order of magnitude)
2/ I assume your "single number" representation would allow all value combinations in all dimensions... in which case WarStalkeR's quick allocation algorithm wouldn't work, his algorithm poses constraints on the allowed combinations in order to not have to brute-force
(or if your representation poses same constraints, then I don't see the advantage over the solution he already implemented)
I already partially implemented slot grouping in my code (still not in the repository, as it is work in progress). And frankly, I feel like it is very, very, very bad idea already. And I will explain why.
Using this code, it is possible to group slots.
- type: STR_THAT_FANCY_HANGAR
crafts: 3
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]
It will group 1st and 2nd 'craftOptions' into group #1, 3rd and 4th into group #2 and 5th into group #3. When slots are grouped, only one of them is allowed to store craft. If craft is stored in 1st of group #1, 2nd slot won't be accessible.
All sounds great, until code attempts to sort them. For example, let's assume I already have one Jetbike (size 25) and one Bicycle (size 5). Despite logically being obvious that there is place for either new Jetbike or Bicycle, code will allow you to get only new Jetbike, because sorting code isn't that smart (it looks sorts by looking for a smallest slot for biggest ship). In addition, once you'll get another Jetbike, game will throw warning message that there are no free slots for Bicycle, because group #1 and group #2 are occupied Jetbikes.
Right now I'm looking into replacing this sorting function with one that utilizes heuristic-based approach, but it might be a little bit complicated and requires more testing, although it shouldn't increase computing complexity by much.
P.S. Meridian, please join Matrix chat, because some code discussions can accelerate process by much.
-
Maybe I understand something incorrectly, or maybe I'm simply wrong, but:
1/ we don't have a problem with space constraints, there is nothing forcing us to use bit maps, we can easily use numbers or even strings... and all 3 internal representations will have similar complexity (on the same order of magnitude)
Maybe I understand something incorrectly, or maybe I'm simply wrong, but:
2/ I assume your "single number" representation would allow all value combinations in all dimensions... in which case WarStalkeR's quick allocation algorithm wouldn't work, his algorithm poses constraints on the allowed combinations in order to not have to brute-force
(or if your representation poses same constraints, then I don't see the advantage over the solution he already implemented)
To avoid a lot of nested if-elses, for example. Though I admit that a numeric range representation could be very similar, considering number positions (hundreds, tens, units) the same role as group bits. Masking and shifting bits is a lot faster than divisions/mod operations, but probably execution speed is not a problem in this game, so perhaps you're right.
-
Why complicate things, when 'std::vector<>' handling in C++ already does everything for you?
I already partially implemented slot grouping in my code (still not in the repository, as it is work in progress). And frankly, I feel like it is very, very, very bad idea already. And I will explain why.
Using this code, it is possible to group slots.
- type: STR_THAT_FANCY_HANGAR
crafts: 3
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]
It will group 1st and 2nd 'craftOptions' into group #1, 3rd and 4th into group #2 and 5th into group #3. When slots are grouped, only one of them is allowed to store craft. If craft is stored in 1st of group #1, 2nd slot won't be accessible.
All sounds great, until code attempts to sort them. For example, let's assume I already have one Jetbike (size 25) and one Bicycle (size 5). Despite logically being obvious that there is place for either new Jetbike or Bicycle, code will allow you to get only new Jetbike, because sorting code isn't that smart (it looks sorts by looking for a smallest slot for biggest ship). In addition, once you'll get another Jetbike, game will throw warning message that there are no free slots for Bicycle, because group #1 and group #2 are occupied Jetbikes.
Right now I'm looking into replacing this sorting function with one that utilizes heuristic-based approach, but it might be a little bit complicated and requires more testing, although it shouldn't increase computing complexity by much.
P.S. Meridian, please join Matrix chat, because some code discussions can accelerate process by much.
Of course, allocation of NEW crafts is the main problem here. If a craft has several possible slots on different hangar types, you don't have only to search for possible free slots but also consider how it affects allocation of the existing crafts. As you could end using a slot which stops acquiring/allocating another craft that, with a different allocation for the in itial, could be acquired.
In my first attempt "Big/small hangars", in which small crafts could be allocated also into big hangars, I solved this forcing allocation of Big crafts first, by ordering array of crafts at bases by size, when a new craft was bought/produced/transfered. But with all different combinations you propose (several different sizes; underwater/aerial; earth/space...) this problem is more complicated (if you consider hangars which could allocate more than 1 type of crafts, and crafts that could be stored in more than one type of hangar)
-
Meridian, I have a piece of great news. Beside implementation of slot grouping (with modding syntax examples above), I've succeeded with code refactor for allocation and sorting functions through heuristic-based approach, and without increasing computing complexity by much.
You can take the look at the code here: https://github.com/WarStalkeR/OpenXcomExMore/commit/9e61c36934c6e318bfb6d48282cb1e74293dbfa6
Sadly, heuristic-based approach isn't perfect and only perfect will be method with backtracking, but it might have huge performance impact. For the most cases, like 99.9% of them, heuristic-based approach should be more than enough.
Although I tested core features (allocation, syncing & etc) for all potential issues. Code still requires polish/update in some other parts that not yet reflect 'optionGroups' change (Analysis and Ufopaedia for sure), but this is less of an issue.
Of course, allocation of NEW crafts is the main problem here. If a craft has several possible slots on different hangar types, you don't have only to search for possible free slots but also consider how it affects allocation of the existing crafts. As you could end using a slot which stops acquiring/allocating another craft that, with a different allocation for the in itial, could be acquired.
I already resolved it. As for how, you can see code itself in latest commit to my repository.
-
I already resolved it. As for how, you can see code itself in latest commit to my repository.
Great news!!! Congratulations!!
I'll look your solution at your Code. I'm very interested! I thought heurístics would be too difficult to implement.
Hope we finally have this hangar feature at oficial repo soon!!
-
1. not the most intuitive syntax for modders
2. mental gymnastics needed to design the number ranges for craft classes
3. easy to fail the mental gymnastics, e.g. too small ranges, resulting in overflow between ranges caused by added weight/size (from weapons)
4. only 2 dimensions... yes, I know, with even more mental gymnastics, you could encode any number of dimensions into natural numbers, but that's honestly too much to ask for
5. inability to define arbitrary/custom combinations, e.g. if land=1-99, water=100-199, space=200-299, I can't make a hangar for land+space crafts
Let's return to issues you've mentioned here.
Unified diff for turning my previous 1-dimensional hangar slots turning into 2.5-dimensional ones: https://github.com/WarStalkeR/OpenXcomExMore/compare/e23a5a0..19b8aa0?diff=unified
Unified diff for implementing slot grouping, heuristic sorting & etc: https://github.com/WarStalkeR/OpenXcomExMore/compare/7aa49dc..19b8aa0?diff=unified
Low: 1
Syntax will be changed for sure. I will do it closer to the weekend.
Mildly annoying: 2, 3
2 - completely avoidable, if modder will decide to allocate only single value to each craft size. 3 - completely avoidable, if modder doesn't add weapons/systems that increase size. In addition, default safety measure 'craftsCanChangeClass: false' is implemented to ensure that craft can't change class. It can be disabled through the modding, if modder has such desire.
Questionable: 4
Pretty much resolved. Now there are 2.5 dimensions without ramping up amount of required mental gymnastics.
Possible deal-breaker: 5
Resolved. Beside slot min/max size, you can group slots (so only one slot from group is used). In addition, all of that is supported by heuristic sorting algorithm that makes sure for 99% that no potential slots or groups are wasted, and all of that without intense or very complex calculations. Analysis screen, Ufopaedia screen, craft refitting screen (and some other parts) also received relevant updates.
-
Added fix: multi-craft hangar destruction now also results in loss of all housed crafts. Do note that if hangar instead of destruction gets only damaged or ruined (without destruction), crafts are transferred to damaged/ruined hangar.
Added feature: in base view, while hovering over multi-craft hangar you can use mouse wheel up/down to switch between crafts that you open in refit screen via right mouse button.
Multi-craft hangar damage/destruction is a little bit dumb right now, it either transfer all crafts to the damaged/ruined version (even if doesn't have same amount of slots) or destroys all crafts if multi-craft hangar doesn't have ruined version.
I will implement smarter version this weekend, where it only moves crafts to slots that "survived" (i.e. available in ruined variant) and destroys all other crafts that didn't have replacement slot for them.
-
When I was working on my version of this feature, some people asked me about the possibility to add another -so
mehow related- feature to exchange two crafts at different bases. At the moment, if you have 2 crafts at 2 hangars in different bases, you need a 3rd hangar to be able to do that exchange -- transfer craft1 to free hangar; transfer craft2 to -ex-craft1 hangar; transfer craft1 to ex-craft2 hangar --
Do you think this could be implemented with your version or would be very difficult due to allocation-rule checking?
-
Do you think this could be implemented with your version or would be very difficult due to allocation-rule checking?
When craft becomes TRANSFER_CRAFT, slot on the receiving side is allocated and it is unavoidable. Anyway, making it possible will require major refactoring of original code and how item transfer is handled.
I'm pretty sure Meridian and Yankes wouldn't want me to touch something that works already perfectly fine.
-
When craft becomes TRANSFER_CRAFT, slot on the receiving side is allocated and it is unavoidable. Anyway, making it possible will require major refactoring of original code and how item transfer is handled.
I'm pretty sure Meridian and Yankes wouldn't want me to touch something that works already perfectly fine.
Yes, I know about TRANSFER_CRAFT management, but it's true that it seems "somehow silly" needing to have 3 hangars to manage 2 crafts going from one base to another -and note hangars take a lot of space in bases-. At original/current code I think it can be done ( a double check at the hangar allocation rules at both bases, discounting the transferred crafts, then a TRANSFER order for boths craft at the same time). With modified hangar rules -mine, your's, Meridian's- which allow more classes and felxibility, that check will be a lot more difficult for sure.
-
Yes, I know about TRANSFER_CRAFT management, but it's true that it seems "somehow silly" needing to have 3 hangars to manage 2 crafts going from one base to another -and note hangars take a lot of space in bases-. At original/current code I think it can be done ( a double check at the hangar allocation rules at both bases, discounting the transferred crafts, then a TRANSFER order for boths craft at the same time). With modified hangar rules -mine, your's, Meridian's- which allow more classes and felxibility, that check will be a lot more difficult for sure.
You need to understand shipping/transfer algorithm of the game. OXCE doesn't have simultaneous transfer between two sides, like X-COM Apocalypse does. All transfer orders/menus are one-sided only. In order to implement the logic you're speaking about, you need to completely rework shipping/transfer code, especially part when you select what to transfer, since it does checks when you change amount and not when you click the "ship/transfer" button. And this is very huge amount of work, which isn't really a priority, not even in the very long run, as there are other more interesting features that are waiting for implementation.
If you're eager, you're welcome to completely rework the transfer/shipping screen and handling, and maybe it can make into OXCE.
-
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:
- 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:
- 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.
-
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
-
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.
-
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.
-
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.
-
This handling flying crafts is good solution
-
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:
- 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:
- 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.
-
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).
-
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).
-
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.
-
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
-
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 :D
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 :D
-
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.
-
[...]
Well, while making submod for XPZ I already drawn various hangars... And I already partially made 3x3 hangars.
(https://i.imgur.com/GH7J0zu.png)
[...]
i really like this, plus those hangars are very cool! Hope. In the meantime it iis possible yo download that submod?
-
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 =)
-
i really like this, plus those hangars are very cool! Hope. In the meantime it iis possible yo download that submod?
Sadly no, since this submod isn't even published and still pretty much work in progress. And it is utterly unbalanced :D
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 =)
Finishing adding the feature isn't problem. Problem is to polish it afterwards to ensure that all edge cases taken into account and you don't end up with some weird behavior, you never intended for. And to ensure that newly added code is as readable as original code.
-
I decided to reorganize all my commits (and how I will handle them in the future), so each of my commits is dedicated feature which anybody can take for themselves.
I've made them independent one from another, except for craft sizes/classification and multi-craft hangars, since multi-craft hangar sorting relies on craft sizes and can't operate without it.
If somebody desires to take inspiration, how specific feature is implemented or just implement it your own code, you can easily do so. Enjoy: https://github.com/WarStalkeR/OpenXcomExMore/commits/oxce-plus-more/
-
The user 0xEBJC has already create some mod-alike for openxcom files (posted here (https://openxcom.org/forum/index.php?topic=11029.msg162004#msg162004) and even avaiable on opencom's mod io page (https://mod.io/g/openxcom/m/facility-expansion-pack-x-com-files)); Dioxine himself is waiting OXCE to support it (https://openxcom.org/forum/index.php?topic=5821.msg163489#msg163489) so we may have still to wait eons to see it? :P
-
so we may have still to wait eons to see it? :P
Because implementing a whole new parameter for crafts and sorting via it is very complicated. I've refactored my craft size and multi-craft hangar code like 5 times, before it became decent looking and with proper sorting.
Just simple code for modifiable soldier/vehicle craft stats that was merged into default branch had to be heavily refactored by Meridian. You even can see the comparison somewhere in the discussion.
-
Because implementing a whole new parameter for crafts and sorting via it is very complicated. I've refactored my craft size and multi-craft hangar code like 5 times, before it became decent looking and with proper sorting.
Just simple code for modifiable soldier/vehicle craft stats that was merged into default branch had to be heavily refactored by Meridian. You even can see the comparison somewhere in the discussion.
ah ok thanks, hope also some good will coder(s) might want to help, in the meantime let's cross our fingers!