Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cargo - Add BI Vehicle in vehicle support #7984

Open
wants to merge 37 commits into
base: master
Choose a base branch
from

Conversation

Vdauphin
Copy link
Contributor

@Vdauphin Vdauphin commented Nov 4, 2020

When merged this pull request will:

  • This PR choose, when possible, to use the BI Vehicle in Vehicle feature (ViV)
  • When loading an item, the function check if it is possible to load it with ViV
    • If yes load it as ViV
    • If no, check if it would be possible to load it if the vehicle is empty
      • Then unload ViV objects loaded and move them to ACE Cargo and put a pallet to mimic the fact they has been stacked.
  • When unloading an item, the function check if the item has been loaded with ViV and spawn it accordingly

To-do:

  • Need to handle the case where player unload a ViV item without ACE Cargo. ie: As driver, you can unload all ViV item
  • Need to differentiate normal "CargoNet_01_box_F" and ACE "CargoNet_01_box_F"
  • Some people need a pallet fitting WW2 (Land_WoodenBox_02_F)
  • Avoid the creation of two parachutes (one from ViV and one from ACE)
  • Test needed for the last paradrop unloading commit

Concept:

Click here

Here the ViV start to be full:
20201104224822_1
Some objects are migrated to the ACE Cargo system and replace visually by a pallet:
20201104224823_1
Here again the ViV is full so some item are migrated to ACE Cargo:
20201104224824_1
Let's continue to load objects:
20201104224825_1
20201104224826_1
Here again the ViV is full so some item are migrated to ACE Cargo:
20201104224827_1
20201104224828_1
20201104225743_1

Example of behavior when a vehicle is already loaded:

A player can load his vehicle. Objects added after through ACE Cargo will shown
20201106220541_1
20201107162853_1

User experience with vehicle loaded with BI action ViV and objects loaded with ACE cargo:

Click here

20211030001530_1
20211030001028_1
20211030001550_1
20211030001605_1
20211030001615_1

Code to test this PR:

Click here

heli = createVehicle ["B_T_VTOL_01_vehicle_F", player getPos [25, 0], [], 0, "CAN_COLLIDE"];

[{ 
    veh = createVehicle ["B_LSV_01_armed_F", player getPos [25, 45], [], 0, "CAN_COLLIDE"];
    item = createVehicle ["Land_WoodenBox_F", player getPos [25, 90], [], 0, "CAN_COLLIDE"];

    // Vanilla loading, like using the scroll wheel
    heli setVehicleCargo veh;

    // ACE cargo loading, like when you interact with ACE menu
    [
        item, 
        heli, 
        true // just true for easy debug
    ] call ace_cargo_fnc_loadItem;
}] call CBA_fnc_execNextFrame; 
truck  = createVehicle ["B_Truck_01_flatbed_F", player getPos [25, 0], [], 0, "CAN_COLLIDE"]; 
[truck, 25] call ace_cargo_fnc_setSpace;

[{ 
    veh  = createVehicle ["B_T_Quadbike_01_F", player getPos [25, 45], [], 0, "CAN_COLLIDE"]; 
 
    // Vanilla loading, like using the scroll wheel 
    truck setVehicleCargo veh; 
 
    for "_i" from 1 to 8 step 1 do { 
        item  = createVehicle ["Land_WoodenBox_F", player getPos [25, 90], [], 0, "CAN_COLLIDE"]; 
 
        // ACE cargo loading, like when you interact with ACE menu 
        [ 
            item,  
            truck,  
            true // just true for easy debug 
        ] call ace_cargo_fnc_loadItem; 
    }; 
}] call CBA_fnc_execNextFrame; 


Community idea:

  • I would suggest the following: Make a setting that if a vehicle has a ViV config, use that config to read cargo space instead of our config value. Handle loading exclusively through ViV. For virtual objects, place a box of the closest size in ViV. Disable vanilla actions for ViV.
  • I like the idea of just displaying a similarly-sized box tbh (to represent the space like you would have in ACE Cargo now): https://github.com/VurtualRuler98/BoxLoader/

@veteran29
Copy link
Member

Need to handle the case where player unload a ViV item without ACE Cargo. ie: As driver, you can unload all ViV item

we could use engine events for ViV stuff. 🙏 @dedmen

@Vdauphin

This comment has been minimized.

@Vdauphin Vdauphin marked this pull request as draft November 4, 2020 15:26
@veteran29 veteran29 added the kind/enhancement Release Notes: **IMPROVED:** label Nov 4, 2020
@Drofseh
Copy link
Contributor

Drofseh commented Nov 4, 2020

I like this conceptually but play mostly with ww2 mods where the modern style of cargo pallet will not be a great look.
Could you include a setting to define a class for the pallet?

@Vdauphin
Copy link
Contributor Author

Vdauphin commented Nov 5, 2020

Do you have a class name for the WW2 pallet?

@Drofseh
Copy link
Contributor

Drofseh commented Nov 5, 2020

Do you have a class name for the WW2 pallet?

I'll have a look for something I think is good, but not everyone will run the same ww2 mods as my unit.
Some people only run IFA3 or only FOW. That's why I would like the setting to accept a class name.

@Drofseh
Copy link
Contributor

Drofseh commented Nov 6, 2020

Land_WoodenBox_02_F would fit, it's a Livonia object too so not mod dependant.

Use ACE own interraction to unload all vehicles as BI action unload all vehicles do.
Vdauphin and others added 4 commits November 7, 2020 09:58
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
This allow player to select the cargo type fitting their current game play periode of history
@Vdauphin Vdauphin marked this pull request as ready for review November 8, 2020 13:27
@dedmen
Copy link
Contributor

dedmen commented Dec 19, 2020

Need to handle the case where player unload a ViV item without ACE Cargo. ie: As driver, you can unload all ViV item

we could use engine events for ViV stuff. pray @dedmen

if you make FT ticket now, and DM me on discord I may be able to get it into 2.02.
I'll be gone for a while now and won't read github :u @veteran29

@veteran29
Copy link
Member

if you make FT ticket now, and DM me on discord I may be able to get it into 2.02.
I'll be gone for a while now and won't read github :u @veteran29

https://feedback.bistudio.com/T155799

@dedmen
Copy link
Contributor

dedmen commented Aug 2, 2021

Btw I completely forgot the ViV events. I'll put them back onto my todo now that I just saw them

@ZluskeN
Copy link
Contributor

ZluskeN commented Mar 8, 2023

Is this blocked by something? I was considering making something like this but it seems the work has already been done.

@BrettMayson
Copy link
Member

@Vdauphin I'm good to review and test? Nothing else to be done here?

@Vdauphin
Copy link
Contributor Author

Yes sure but it definitely need some testing in "real" condition to find some edge cases

@LinkIsGrim LinkIsGrim self-requested a review July 4, 2023 19:10
@dedmen
Copy link
Contributor

dedmen commented Jul 14, 2024

Seems we are still missing: #7984 (comment)
The engine events were added 3 years ago. But they are not used in this code.

@dedmen
Copy link
Contributor

dedmen commented Jul 16, 2024

Seems we are still missing: #7984 (comment)

But since (I completely missed this change back then)

No because it will unload only vehicles and not items/objects... Also the interaction is not show if only item/objects are loaded

And https://github.com/acemod/ACE3/pull/7984/files#diff-19cb528ab9cffb1ff448fcf3c1125e9fb154cfa1c9b49ca23cc7b27ac643f3ffR4
We don't need it anymore, because there is no concern about them being unloaded through non-ace means?

But there is still atleast one unhandled way to unload a box.

You can ACE-Carry pull it off the back of the truck.
IMO that's amazing and immersive, but is that handled? Skimming through the code I couldn't find anything about it.

Okey I tested it
We display cargo items in UI by GVAR(loaded), when you pull a box off a truck with ACE Carry, the loaded variable is not updated.
The item shows in cargo, even though its not on the vehicle.

Repro:
Load 3 medical boxes onto hemtt flatbed
Check cargo menu of truck, shows 3 boxes and a wheel.
Walk close to the side of hemtt, grab box with ACE carrying off of the back of the truck.
(Diag binary throws an assert, the truck doesn't like that a vehicle in its cargo, is not attached to it. This is not really a problem, Arma will fix itself up next frame and detect that something was detached. I'm not sure if manually detaching would help here, or whether unloading with setVehicleCargo might cause issues)
Player is now carrying box (its way too high up, but can just be scrolled down normally)
Drop box onto ground.
Check hemtt cargo menu, shows 3 boxes and a wheel. But only 2 boxes are on it.
Unload the other boxes.
Truckbed is empty, but still shows 3 boxes in cargo.
Open cargo menu, and unload box from there (A box that isn't actually there anymore)
The 10s timer runs and, it says you successfully unloaded the box. But there is nothing in your hand. It now disappeared from the cargo menu.

IMO we should keep this carrying feature and fix it up instead.

cursorObject addEventHandler ["CargoUnloaded", {
	params ["_parentVehicle", "_cargoVehicle"];
hint str _this;
systemChat str _this;
}];

This does NOT detect the unload. Because Carrying doesn't detach.
It does detect the ACE Cargo unload.
And it does NOT detect when you detach cursorObject the box from the truck.
But objNull setVehicleCargo cursorObject works.

IMO the detach is a bug. The vehicle will notice that the object isn't attached in the next simulation cycle and remove it from the internal list. The Eventhandler should fire there too when it notices it missing. I will fix it for 2.18.
But maybe we could handle this better if we had a general detach eventhandler?

The easy fix would be to use this eventhandler, and inside carrying, check if the thing you are picking up, is currently attached to anything, and if yes then detach it from that first.

whether unloading with setVehicleCargo might cause issues

It does. https://community.bistudio.com/wiki/setVehicleCargo

Unload specific loaded vehicle (will paradrop if dropped from high altitude)

When on some platform high in the air, even when not flying and standing on some platform, this could be causing issues.
I'd rather not have a sudden parachute attached to something you're carrying.

@johnb432 johnb432 self-requested a review July 16, 2024 11:45
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've reviewed the PR in it's current form and before I merged master into it recently.

The cargo net idea is neat, but it's the cause of several headaches:

  • If the cargo net is unloaded via ViV (but not ACE Cargo), something should happen: Either it's replaced and the unloaded cargo net is deleted (making it impossible to unload cargo nets essentially) or the space left behind should be filled up by some of the contents of said cargo net. Currently nothing happens.
  • Generally, when you unload something, the space left behind is not filled by items inside ACE cargo, which imo it should replenish - I can address that though. I'm a bit hesitant about this suggestion though, since you might want to load something ViV that isn't ACE cargo and if the thing replenishes automatically, you'll need to unload all of the stored ACE cargo (and some ViV cargo) before being able to store your ViV item. Thought about this again, not a good idea, so I won't implement it.
  • I'm trying to use the ViV EH, but given that ACE very dynamically loads & unloads stuff from ViV, I'm not sure how to prevent those EH from triggering when ACE is doing the loading/unloading vs a 3rd party.
  • Bug: Once you have filled up all ViV spaces (including using the cargo nets), adding another cargo item will make the "loose" cargo items (the ones not in the form of a cargo net) go into ACE cargo, where they can't be seen.
    E.g.: Load numerous wooden boxes into a flatbed HEMTT until you have 3 cargo nets (you might have to increase the cargo space of the vehicle). Load several more wooden boxes - the first ones will stay, but after a certain threshold the wooden boxes will disappear, being moved to ACE cargo. You can start loading more boxes and the cycle begins again.

If we get rid of the cargo nets, it would make things easier, as the only time something can be unloaded is when something is purposely unloaded and not put into ACE cargo immediately. We could use the CargoUnloaded EH and check is something is in the loaded array of a vehicle - if it is, remove it from the array.

It would also remove the cargo-net-type setting. I really don't like it being a setting, imo it should be config, but even better is if we don't need it at all.

One more thing: Do we want to have the "unload all vehicles" action unload all ViV, including ACE cargo or do we exclude ACE cargo from that (PR has the latter implemented already)?

Thoughts?

@dedmen
Copy link
Contributor

dedmen commented Jul 17, 2024

New entity events in 2.18
Attached [attachedObj, parentObj, isReattach bool]
Detached [attachedObj, parentObj]

Detached also fires on ViV cargo unload. And could also handle the ACE carry issue. (tho it would throw asserts in diag and I don't like that :D)
We should be pushing a dev-branch next week, if you want it early for testing I can send it on discord.

which imo it should replenish

Considering how many headaches it causes. Also there might be cases where you don't want it.

Say you take a box from the front out, to interact with it. And then you put it back in. It can go into the same spot as it was before.
If it replenishes, and some tiny object like a jerry can, takes the space where previously the big box sat at. And now the box doesn't fit onto ViV space at all and must go into hidden cargo.. Meh.

I think I would have more situations where I don't want to replenish.
Could make a button in Cargo UI to do it manually, but UI edits are more work.
Can already do it manually anyways, just need to unload and re-load.

@johnb432
Copy link
Contributor

johnb432 commented Jul 17, 2024

New entity events in 2.18 Attached [attachedObj, parentObj, isReattach bool] Detached [attachedObj, parentObj]

Detached also fires on ViV cargo unload. And could also handle the ACE carry issue. (tho it would throw asserts in diag and I don't like that :D) We should be pushing a dev-branch next week, if you want it early for testing I can send it on discord.

I believe I've resolved the ACE carry issue in 623e77d by just unloading from ViV if they are loaded in ViV.
Currently I haven't added a ViV unload EH to handle ViV unloading in such a manner, but I'm waiting for feedback on what I've written above first before I push anything more.

Considering how many headaches it causes. Also there might be cases where you don't want it.

Say you take a box from the front out, to interact with it. And then you put it back in. It can go into the same spot as it was before. If it replenishes, and some tiny object like a jerry can, takes the space where previously the big box sat at. And now the box doesn't fit onto ViV space at all and must go into hidden cargo.. Meh.

I think I would have more situations where I don't want to replenish. Could make a button in Cargo UI to do it manually, but UI edits are more work. Can already do it manually anyways, just need to unload and re-load.

Yea, I won't add it then.

@johnb432
Copy link
Contributor

johnb432 commented Jul 18, 2024

The more I think about it, the more I think this is whole PR is a bad idea as it's implemented currently. Please don't get me wrong, I really like the visual effect this adds, but I don't think it's done right.

One more thing: Do we want to have the "unload all vehicles" action unload all ViV, including ACE cargo or do we exclude ACE cargo from that (PR has the latter implemented already)?

At the moment, if you load something, you have no control over if it goes into ViV or ACE cargo space directly. As the PR is currently, if you use the "Unload all vehicles" action, it will unload all ViV items except items that are part of ACE cargo, although for some reason it unloads the cargo nets (at least according to the code - I'm guessing it's a bug, if not, unexpected behaviour). As dedmen pointed out before:

This interaction unload only vehicles not loaded inside ACE cargo

Isn't that confusing? you select "unload all" and it only unloads half of the things 🤔

I agree: It's confusing and inconsistent, especially seeing as at the moment you have no control over anything. The "unload all vehicles" action should truly unload all ViV items imo.

Solution to that would be to add a button inside the cargo menu that would allow items to be moved from ViV to ACE cargo space and vice versa. This would allow one to set specific items to be ACE cargo and others to be ViV.
Unfortunately I can imagine there being some hurdles to overcome, such as the CargoUnloaded EH firing and thinking the item was unloaded and therefore removing it from the list, whereas in reality it was just moved. I guess we can use some flags to deal with that, but honestly I'd prefer not having to.
Furthermore, if you want to use ACE cargo only (say you want to keep the ViV space free in case of an emergency pickup of whatever), you would have to move every single item you load into ViV into the ACE cargo space. Depending on how many items you have loaded, this is tedious work. I guess a setting for preferred cargo space could be added to remedy that though.
Finally, I have mixed feelings about mixing ViV and the ACE cargo mechanics, which work in different ways.

So: Why not keep both mechanics separate instead?

The idea I have in mind is to have two loading options for each item: one for regular ACE cargo and one for ViV if it exists. The ViV items added to vehicles wouldn't be stored as ACE cargo and would only use regular ViV mechanics, it would be completely independent of the ACE cargo system. This means that the "Unload all vehicles" action would remain as it is in Vanilla and truly unload all ViV objects.
To unload ViV items, either adding a new interaction with the same style of menu as the cargo menu or adding a button in the cargo menu to switch to ViV items and back are the ideas I had in mind (first suggestion is probably easier to do, but not as elegant as the second). The nice thing is that you could probably use the "unload" and "deploy" buttons as they are now (unload would let the game engine handle unloading, deploy would use the existing ACE cargo deployment system). I imagine ACE cargo's paradrop mechanic would also be reused if necessary.

Going with this suggestion would also avoid the whole cargo nets system.

@johnb432
Copy link
Contributor

johnb432 commented Jul 19, 2024

So: Why not keep both mechanics separate instead?

I've come up with a first draft of this idea, see https://github.com/acemod/ACE3/tree/cargo-add-viv/addons/cargo.

  • ACE cargo spaces and ViV cargo spaces are fully separate.
  • To access the ViV cargo space, you go through the into the ACE cargo menu and press the bottom left button to switch from one cargo space to another.
  • You can deploy and unload ViV items. ViV cargo that isn't ACE cargo compatible can't be unloaded via this menu (e.g vehicles).
  • Missing: Handling of paradropping ViV cargo items, but that's easy enough to do. I just don't want to spent more time atm getting this thing fully functional, only to find out that nobody wants it.

Thing to sort out if people want this version:
Atm, the ViV system uses some ACE cargo config attributes, such as size and canLoad (size is used to determine if an item is not loadable (-1 would be not loadable)). However, there are Vanilla attributes (https://community.bistudio.com/wiki/Arma_3:_Vehicle_in_Vehicle_Transport) such as canBeTransported that exist.
Do we fully want to separate ViV cargo from ACE cargo (could make it a separate component at that point) and make interactions follow Vanilla rules only or do we make/keep a somewhat hybrid system?


Regardless of which version we adopt, I noticed that the main interaction points of ViV cargo are quite low and sometimes interfere with the main interaction point of the vehicle. Culprits are ace_interaction_fnc_getVehiclePos and ace_interaction_fnc_getVehiclePosComplex, which limit the height of interaction points to the height of the eyes, to avoid interaction points being too high up. Any suggestion on how we can improve interacting with ViV cargo, without causing issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants