Author Topic: [DONE][Suggestion] Alternate loadout system #2  (Read 8093 times)

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9106
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #15 on: January 08, 2023, 12:53:15 pm »
The other thing is only indirectly related to my changes. It's about the base inventory screen rather than the craft inventory screen. In my changes to the craft inventory screen, equipping items will add those items to the craft. So I think it would be good if the base inventory screen did the same thing. i.e. if you add items to the equipment of a soldier from the base inventory screen, then those items should be added to whichever craft the soldier is currently on (if they are on any craft at all). I have a vague sense that Meridian was going to implement this himself; but maybe I imagined that. In any case, I think it would be an improvement consistent with the changes that I already made.

... and then follow up with the base inventory change afterward if we're in agreement about that.

is this effectively the same as this: https://openxcom.org/forum/index.php/topic,10984.0.html

if yes, I have already implemented that, please have a look: https://github.com/MeridianOXC/OpenXcom/commit/cd932f6885fb86ae9ef4763ca75f15399472713d

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #16 on: January 09, 2023, 12:36:14 am »
is this effectively the same as this: https://openxcom.org/forum/index.php/topic,10984.0.html

if yes, I have already implemented that, please have a look: https://github.com/MeridianOXC/OpenXcom/commit/cd932f6885fb86ae9ef4763ca75f15399472713d

Yes. That's what I was thinking about; and at a glance, the implementation looks good. (I don't like the variable name `_backup` though. I'd go with `_soldierCraftMap` or something like that.)

So I guess everyone will be very happy. :)

Offline UchuuKaizoku

  • Squaddie
  • *
  • Posts: 1
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #17 on: January 12, 2023, 06:59:00 am »
After talking with karadoc about how I think equipped items should be considered "reserved" I realized there isn't really any reason this feature should be reserved to the alternate equipment management.
So, working from karadoc's commit I've added two buttons to the craft equipment screen labeled for a lack of better names "+" and "-":
  • Plus button works the same as karadoc's version showing you all the items in the base
  • Minus button does the same except it filters out equipped/"reserved" items i.e. no more accidentally stealing equipped items
  • Inventory button keeps the old behaviour
Inventory hotkey uses old behaviour normally and Minus behaviour when alternate equipment management is enabled.

github.com/MeridianOXC/OpenXcom/compare/oxce-plus...Uchuu-Kaizoku:OpenXcom:inventorydev

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #18 on: January 16, 2023, 07:02:40 am »
What I want to say is, original requirement and your requirement are not compatible.
What you are proposing is not an improvement of an existing system, it is a different system.
To be honest, when I first read this post I was a bit unsure what you thought the original requirements were. Since the original request was marked as 'complete', I thought the feature was already working as intended, and that we were just trying to refine it a bit.

But after talking to UchuuKaizoku, I realise that that you weren't just going for a system loosely inspired by apoc. It was literally meant to have soldier equipment strictly reserved so that items were effectively stored on the soldier rather than in the base or the craft. I didn't realise that was the aim, because I don't think the current implementation achieves it - and I prefer the current implementation compared to that strict goal anyway!

So Meridian, you were right. I probably was proposing a different system. A system that is very similar to what is currently implemented, but not the same as the 'apoc' idea that was originally intended. I think this whole discussion is a bit foggy, with different people having different understandings of how things currently work, and different ideas of how things should ideally work.

I've taken a bit of time to write some notes about what I think the three different systems are. Option 1 being standard xcom, option 2 being something close to what we have currently, and option 3 being the strictly reserved soldier equipment system. I'll paste those details in a moment. But let me just say I thought option 2 was more-or-less what everyone was aiming for here and I was enjoying using that system. I'm reasonably happy with what we have currently implemented. But I have no interest in using option 3 whatsoever, and I think it could be a bit of a nightmare to implement and maintain. So although the original idea might have been something closer to option 3, I'd urge people to think carefully about what they actually want. It would be a shame to spend a lot of effort 'fixing' discrepancies in the current system only to end up with something less useful.


Here's are the details that I've written down for the three options:
(`*` means implemented already. `#` means currently unimplemented. `?` means the current implementation is ok, but I have a suggested change.)


Option 1: standard
aka: ignore soldier equipment
-------------------------------------------------------------
* (Standard original OXCE behaviour)
* Soldiers' remember what items they last used, but do not bring any items with them or reserve items for any reason.
? Inventory changes from the base screen, the craft screen, or pre-battle are saved - updating the soldier's remembered items list. (Although, perhaps it would be better to not save pre-battle changes - just for consistency with the other options.)

* Items can be sold, transferred, or used in any way - as normal - with no special restrictions.

* Moving soldiers does not trigger any movement of items.
* Craft item template save/load affects all items on the craft, including any items equipped by the soldiers.
* Craft inventory screen shows only the items that are currently in the craft.




Option 2: the currently implemented 'alternative system'. (This is my preference.)
aka: request soldier equipment

-------------------------------------------------------------
* Soldiers' remember what items they last used. This is their 'preferred equipment' list.
* Inventory changes from the base screen or the craft screen are saved as the soldiers' current 'preferred' equipment. Changes made in pre-battle, or during battle, are not saved.

* Items can be sold, transferred, or used in any way - as normal - with no special restrictions.

* Moving soldiers on / off a craft will also move their preferred equipment list with them (if the items are available in the base).
* Equipment of the soldiers on the craft cannot be separately moved off the craft. (They must be either unequipped first, or the soldier must be moved off the craft.)
* Items in the base's storage can be added to the craft, regardless of whether they are equipped by other soldiers in the base.
# Craft item templates save/load affects only the unequipped items on the craft. (i.e. templates refer to the craft's extra items, not including the preferred inventory of soldiers currently on the craft.) (Edit: I've got other thoughts about this now, so it might be worth ignoring this one, or discussing further).
* Craft inventory screen shows all items available in the base - including items by soldiers not on the craft. (Items can therefore be 'shared' by multiple soldiers. i.e.  Two or more soldiers can have the same unique item in their preferred equipment list.)
* Changes made to soldiers' equipment from the base screen or the craft inventory screen will result in equivalent changes to craft items.
? When a soldier is moved onto a craft, if there were not enough items in the base to add the full equipment list to the craft, the missing items should not be removed from the craft when the soldier is removed. i.e. Adding a soldier to the craft then immediately removing the soldier should not have any net effect on the craft's equipment list.




Option 3: strict apoc
aka: reserve soldier equipment

-------------------------------------------------------------
* Soldiers' remember what items they last used; and they are treated as literally 'holding' these items at all times. The items are strictly reserved, preventing for all other uses.
* Inventory changes from the base screen or the craft screen are saved as the soldiers' current 'reserved' equipment. Changes made in pre-battle, or during battle, are not saved.

# Equipment held by soldiers cannot be sold, transferred, researched, or used in manufacturing. Items held by soldiers should not appear in the base storage screen, with the exception of the 'force sell' screen when storage is overfull.
# Transferring a soldier to another base also transfers their held items.
# When a soldier is injured, they drop all items. (This is a technical issue only. The inventory screen is unavailable for injured soldiers.)

* Moving soldiers on / off a craft will also move their equipment with them.
* Equipment of the soldiers on the craft cannot be separately moved off the craft. (They must be either unequipped first, or the soldier must be moved off the craft.)
# Equipment of the soldiers not of the craft cannot be added to the craft. (The items are reserved by the soldiers still in the base, and therefore should be unavailable for the craft. We cannot share or steal the equipment of other soldiers.)
# Craft item templates save/load affects only the unequipped items on the craft. (i.e. templates refer to the craft's extra items, not including the preferred inventory of soldiers currently on the craft.)
# Craft inventory screen shows all items available in the base - excluding any items held by soldiers not on the craft. (Items can not be shared by multiple soldiers. If a soldier has a unique item in their equipment list, then no other soldier can equip that item.)
* Changes made to soldiers' equipment from the base screen or the craft inventory screen will result in equivalent changes to craft items.


----

Implementation notes:

* The current version of the game stores an inventory list for every soldier, but these items are not actually stored by the soldier. The soldier's items are always stored by the craft or base. Therefore, the 'strict apoc' setting must explicitly check the items list of each soldiers to manually reserve those items from other usages. This makes implementing the strict setting (option 3) a bit tricky, and potentially creates a code maintance issue - since every type of item usage / transfer must always check these item lists.

* Regarding the last dot-point in option 2, about moving soldier on and off craft. This is what we were discussing earlier in this thread. For me, it is important that moving a soldier onto a craft and then off the craft again should not result in any net change. And to ensure this, we might need to store the craft's "extra" items as a separate list, so that they can be remembered even when some of those items are currently being used for a soldier's inventory. We could simply ignore this, and the result would be a bit of minor inconvenience in the craft inventory changing sometimes due to not enough spare items; but I think it would be nice to fix it. The issue doesn't happen in option 1, because soldier's don't bring their equipment; and it doesn't happen in option 3 because the items on the craft would not be allowed to include missing items from the soldier's inventory in the first place.



And finally, let me just clarify that I'm not really requesting that we have a three-option system implemented in the game. We could do that and it might be good, but I don't really mind what happens. I'm just trying to clear up some confusion about what I was aiming for and what other people might be aiming for. If the goal is just to have the standard system and 'reserved soldier equipment' as an alternative system, then that's fine - but I'll just stay out of it. I'm only really interested in using option 1 or 2.
« Last Edit: February 14, 2023, 08:28:05 am by karadoc »

Offline R1dO

  • Colonel
  • ****
  • Posts: 442
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #19 on: January 20, 2023, 10:54:23 pm »

Since this seems to be the place about putting up (new) ideas for the craftEquipment screen I'd figured why not show the system I currently use.
It has evolved over time but is currently in a state for which i believe could benefit others as well.


The basic premise for this design was to allow for GUI adaptions but not for any changes to the underlying logic related to item allocation to soldiers.
New query methods were allowed if needed for numbers (but thanks to then new apoc-style equipment those are no longer necessary).

The main goal was to make item allocation a bit easier.
The true power lies in those situation where a mod provides a huge number of battleItems but stock is (always) limited.

See attached image-collage to get an ideas of how those changes look and the sections below for a description.

If interested in (part of) those ideas I can put up a demo branch rebased upon latest oxce-plus (9a90fab43).
The code gates most changes behind an user option but there are some refactors.
Reason for the gating is to let impacted code stand out while minimizing changes to existing logic and UI.


Major visual changes (A-C).
===========================
(A)
Changes the info line **if** the craft has a limit on item amount and/or size.
All fields (except for "Soldiers>##") are running numbers responding to list changes.

Intention behind this change:
Make player aware of the limitations before any error pops up.


(B)
Introduces buttons that operate on the list as visible (e.g. after filter has been applied).
They accept the same input as existing list button (e.g. respond to LMB, mouse-wheel and RMB).

There are two important things to note about the usage of those buttons though:
1) Those buttons use a 2-step approach when changing numbers.
   - The first click tries to balance claimed items in the direction chosen
     + Pressing ">" only operates on items that have less assigned to craft than claimed by soldiers.
     + Pressing "<" only operates on items that have more assigned to craft than claimed by soldiers.
   - Only when the list is fully balanced the buttons will operate on the full list again.
2) When using these buttons any error message related to craft limits is suppressed.
   - Allows to (temporarily) assign all items to craft in order to assist equipping via inventory button.
   - Even though errors are suppressed they are still enforced, see [F] how that is handled.

Intention behind this change:
Make item allocation easier but have some safeguard against accidental removal of claimed items (without enforcing a minimum).
(and visual AMSR while holding down LMB. But that is pure coincidence ... or is it? :))


(C)
Adds an extra column to show if (and how much) of a soldier claim exist on a item.
This new column uses a (translatable) indicator to convey the status, more_than/exactly/not enough, followed by the claimed amount.
The extra column approach was chosen because I did not want to change the meaning of the "on-craft" column.

Intention behind this change:
Inform player there are claims on certain items.

Side-note:
Image as shown still has an old alignment for the 'on-craft' and 'claimed' columns
Since the creation of the image those are shifted to the right (closer to original location of on-craft).
There still is an offset to the left of ~1 character (6px) though, to account for "more than reasonable" numbers.


Other changes (D-F).
====================
(D) Combobox.
-------------
This button now responds to RMB-clicks, when that happens the behavior changes into:
+ Show the inverse of the selected category (anything not belonging to ...).
+ Change combobox text to reflect this inverted list display.

The second change is that the default filter categories learned a new one.
- A filter to only show items claimed by soldiers.

(E) Visit inventory button.
---------------------------
This button changes appearance if it is detected that some claimed item is missing on the craft.
If that is the case any inventory visit would mean that some soldiers lose said item(s).

When the button is in this warning state an LMB click will show a warning.
This warning tells why it is not a good idea to visit inventory at this moment (and how to forcefully visit anyway).
A visit to inventory can always be enforced using 'CTRL+click' or 'RMB'.

(F) Return to base (ok) button
------------------------------
This button changes appearance if it is detected that some craft item limit has been exceeded.
A click on this button will replay any stored warning messages and it is not allowed to exit this screen.

To prevent players getting stuck on this screen a shortcut has been provided (CTRL+ALT+click) for forced exit.
This forced exit will restore craft inventory to the state from before entering this screen (to prevent erroneous craft state).


Relation with 'oxceAlternateCraftEquipmentManagement`
=====================================================
The original intent of this redesign was to provide extra info (and QoL operations) using existing data.
Since apoc-style predates this suggestion care has been taken as to not interfere with it's logic (only it's visuals).

This means that any protection provided by apoc-style is not overridden by this redesign (e.g. very unlikely to see "< ##").

It also means some gratitude is in order. This functionality allowed me to reuse some logic and thus made life easier.
« Last Edit: January 20, 2023, 11:00:14 pm by R1dO »

Offline Scamps

  • Captain
  • ***
  • Posts: 86
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #20 on: January 21, 2023, 12:34:29 am »
Did a brief test with both Meridian commit and Karadoc commit - both are welcome changes!
Thank you, Karadoc, for taking time and writing down options. I agree that option2 is the best. (I also agree that inventory code is... complicated and big changes are best left to devs  :) )
//I just learnt about craft templates reading this thread.

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #21 on: January 21, 2023, 01:20:45 am »
Thanks for checking it out, Scamps.

And R1dO,
All of that UI work looks quite good to me. I like that the < and > buttons effectively have normal vanilla functionally, but with a clear and intuitive 'interpretation' in terms of soldier equipment & craft equipment. That similar to what I had in mind earlier, but perhaps better. I also like that the inventory button had a visual indicator for when it going to screw up my soldiers' inventories. The "!!!" doesn't look super attractive - but at least it is a clear warning, and I don't have a better idea. On the combo box, I don't know how 'claimed by soldiers' is different from 'equipped' (but I don't tend to use the 'equipped' thing, so maybe I'm wrong about what it does).

In any case, I'd pleased to see all of those challenges implemented alongside the alternative inventory system. I like it. But for the original equipment system (i.e. normal / 'option 1' / 'ignore soldier equipment'), I don't think there is a concept of soldiers claiming equipment - so I'd only want the additional visual information and warnings about claimed equipment to be activated when using the alternative system.

Offline R1dO

  • Colonel
  • ****
  • Posts: 442
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #22 on: January 21, 2023, 01:48:39 am »
Thank you for the kind words.
I do  believe my concept is very close to your "option 2" proposal (without the automation or protection).

Any new text (like "!!!") is actually translatable (choose your own poison ;)).

As for the difference between "equipped" and " claimed". It also took me a while to grasp the former. For your info:
- Equipped means: items assigned to craft inventory
- Claimed means: items claimed by soldiers.

As for the concept of soldiers claiming.
It does not really exist in my concept (other than that it made sense for variable names). It just makes use of something that was already tracked by OXC (e.g. latest stored soldier equipment, in the code it is called "game managed layout").
This concept only works in the confines of the mentioned screen.

As for implementation.
Even though it sounds nice to have any idea implemented, in the end it has to be maintained by the (main) devs.
A multitude of options is detrimental in that regards, hence why I put it up as a possible idea.
If it gets traction I'm willing to discuss how it can be merged with existing ideas / change my code to make it conform. But I can understand if the proposed change is deemed to invasive for inclusion.
That is one of the reasons i did not propose it earlier, too uncertain if it did not deviate too much from vanilla.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9106
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #23 on: April 11, 2023, 11:41:37 am »
The behaviour is what I described earlier (also described in the commit message on github).
The gist of it is that if go into the inventory screen from a particular craft, you'll see *all* of your base items. And if you change any soldier's equipment from that screen, those changes will be reflected in the items that are loaded onto the craft.

The way this is implemented is by temporarily moving all base equipment on the craft, and then moving it off again after the changes have been made.


The moving of all base items into the craft only works when the "Everything" filter is selected (and no quick filter is used).
(otherwise not displayed items are not moved)

Is that intended?

If not, should I hide the "Inventory" button when the combobox filter or the quick filter are used?

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #24 on: April 11, 2023, 03:13:12 pm »

The moving of all base items into the craft only works when the "Everything" filter is selected (and no quick filter is used).
(otherwise not displayed items are not moved)

Is that intended?

If not, should I hide the "Inventory" button when the combobox filter or the quick filter are used?
It's a bit quirky, but it is intended.

The main use-case is when preparing for special equipment restricted missions. For example, the player might want to filter for 'underwater' so that they don't have to remember / double-check which equipment will be available underwater while they are looking at the inventory screen. The filter is not applied items that are currently equipped though, so could potentially be confusing. (I still think it's best to not filter out the currently equipped items, because otherwise the player might mess up all their equipment by accidentally opening the inventory with a search / filter active.)

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9106
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #25 on: April 11, 2023, 04:01:26 pm »
OK, thanks.

I have split the commit into 2 commits and uploaded on the `test` branch:
https://github.com/MeridianOXC/OpenXcom/commits/test

I have done some restructuring of the code, but hopefully without any unintended functional changes.
The only intended functional change was that I excluded also items with `ignoreInBaseDefense: true` (on top of you excluding the vehicles/HWPs). For example all those blue and red chips.
Please review the code, maybe you'll see something I messed up.

If you can, I would appreciate also help with testing.
A test build is available here: https://lxnt.wtf/oxem/builds//ExtendedTests/Extended-7.8.16-test-4f2fe9402-2023-04-11-win64.7z

Offline karadoc

  • Colonel
  • ****
  • Posts: 232
    • View Profile
Re: [SUGGESTION] Alternate loadout system #2
« Reply #26 on: April 12, 2023, 02:28:16 am »
I've read the code, and I can't see any problems with it. I agree that the `ignoreInBaseDefense` thing is  good idea; and the restructuring looks good. I left a couple of very minor comments on github.

I haven't test it yet though. I'll let you know if I notice any weirdness.

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9106
    • View Profile
Re: [DONE][Suggestion] Alternate loadout system #2
« Reply #27 on: April 14, 2023, 03:11:51 pm »
Merged into OXCE 7.8.18

Offline Meridian

  • Global Moderator
  • Commander
  • *****
  • Posts: 9106
    • View Profile
Re: [DONE][Suggestion] Alternate loadout system #2
« Reply #28 on: April 29, 2023, 04:22:10 pm »
Adding links to 2 related changes:

1. Improvement to loadout via base (not via craft): https://openxcom.org/forum/index.php/topic,10984.0.html
2. Option "add on top" when loading craft equipment: https://openxcom.org/forum/index.php/topic,10851.0.html

Offline psavola

  • Commander
  • *****
  • Posts: 839
    • View Profile
Re: [DONE][Suggestion] Alternate loadout system #2
« Reply #29 on: May 01, 2023, 04:39:34 pm »
After the base has a certain amount of battlescape items, the rest don't show up when you try to equip soldiers. You'll have to add them to the pile manually on the craft screen, for example. This started appearing with an X-Com Files campaign as it progressed and the number of eligible battlescape items on a base increased; currently, there are already about 10 screens worth of equipment to select and the number is only liable to increase.

I suppose there is some limit, buffer or some such that doesn't quite work if there are way too many different kinds of battlescape items on a base.

In the attached save, you can see this when equipping a craft in EU base, while Brazil base is working fine.