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 - Improve various aspects #9617

Merged
merged 18 commits into from
Nov 17, 2023
Merged

Conversation

johnb432
Copy link
Contributor

@johnb432 johnb432 commented Nov 3, 2023

When merged this pull request will:

  • Code cleanup.
  • Moved event "ace_cargoLoaded" from being called within event "ace_loadCargo" to being called in FUNC(loadItem).
  • Added icon to renaming action (icon is from base game).
  • Deleted FUNC(handleDeleted), as it shared close to the entire code with FUNC(handleDestroyed).
  • FUNC(handleDestroyed) is now triggered when an entity is killed or deleted. It deletes all loaded cargo items of a vehicle. It also checks if item was in a vehicle as cargo and removes itself from said vehicle's cargo.
  • Deleted FUNC(validateCargoSpace), as it serves no purpose.
  • Added config case verification to various functions. This should remove any case sensitivity.
  • Changed a couple of condition checking instances, such as:
private _hasCargoConfig = 1 == getNumber (configOf _x >> QGVAR(hasCargo));
private _hasCargoPublic = _x getVariable [QGVAR(hasCargo), false];
(_hasCargoConfig || {_hasCargoPublic})

is now

_x getVariable [QGVAR(hasCargo), getNumber (configOf _x >> QGVAR(hasCargo)) == 1]

They aren't quite the same, but I feel the updated version is what is intended.

  • FUNC(canUnloadItem) received more conditions to check.
  • Fixes ACE Cargo actions don't show up under special circumstances with JIP #9109 by having FUNC(initObject) be executed every time as global and JIP when its size is changed.
  • Applied the same fix to FUNC(initVehicle), as it also had the same problem.
  • The variable set on vehicles QGVAR(space) is kept up to date more rigorously and accurately. The integration of negative sizes of items has been improved.
  • The cargo menu has been enhanced:
    • When an item is not unloadable (= its size is negative), the item will be displayed in red in the menu,
    • Added tooltips showing the cargo size of an object or if it's not unloadable.
    • Does more checks for invalid states (e.g. if the vehicle has been locked or if there is any cargo left).
  • Some files in common were also cleaned up, as they were used.
  • In dragging, FUNC(dropObject_carry) was improved so that it no longer drops the carried item if it's possible to load the carried item.
  • Some settings files outside of cargo had their categories changed, as localized vs. unlocalized entries, despite resulting in the same display name, are not considered to be the same category in CBA settings. If the categories in cargo had only been changed, there would have been two ACE Logistics categories in the CBA settings.
  • Improved documentation.

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@rautamiekka
Copy link
Contributor

Damn, that's a lot.

There are so many simple checks behind lazy evals, many even in succession, that will surely do more harm than good, like the (_hasCargoConfig || {_hasCargoPublic}) in the OP or booleans similar to alive ACE_player, though.

@LinkIsGrim LinkIsGrim added kind/enhancement Release Notes: **IMPROVED:** kind/bug-fix Release Notes: **FIXED:** labels Nov 4, 2023
@LinkIsGrim LinkIsGrim added this to the Ongoing milestone Nov 4, 2023
@LinkIsGrim LinkIsGrim self-requested a review November 4, 2023 01:14
@LinkIsGrim
Copy link
Contributor

GVAR(interactionVehicle) and GVAR(interactionParadrop) variables keep their values after the menu is closed, this can be problematic.

Found a bug when cancelling item load but trying to get consistent repro atm.

@LinkIsGrim
Copy link
Contributor

LinkIsGrim commented Nov 11, 2023

Carry item -> Load with LMB -> Cancel load: Item remains attached to player, carrying hint is gone

@johnb432
Copy link
Contributor Author

johnb432 commented Nov 11, 2023

GVAR(interactionVehicle) and GVAR(interactionParadrop) variables keep their values after the menu is closed, this can be problematic.

In what way? For GVAR(interactionVehicle) I can understand, but not for GVAR(interactionParadrop), as it's a bool that is in either of two states.

addons/common/define.hpp Outdated Show resolved Hide resolved
@johnb432
Copy link
Contributor Author

johnb432 commented Nov 11, 2023

#9493 (review) is still relevant. Unfortunately no text is displayed, so this issue should be resolved before being merged.

@johnb432
Copy link
Contributor Author

#9493 (review) is still relevant. Unfortunately no text is displayed, so this issue should be resolved before being merged.

I'm getting an error code 5. It happens randomly when you repeatably try to paradrop an item.

My guess is that the cargo menu doesn't close fast enough and EFUNC(common,progressBar) does not like that. I'll try delaying it by a frame.

@LinkIsGrim LinkIsGrim modified the milestones: Ongoing, 3.16.2 Nov 17, 2023
@LinkIsGrim LinkIsGrim merged commit d1f0dc5 into acemod:master Nov 17, 2023
5 checks passed
@kymckay kymckay mentioned this pull request Nov 22, 2023
@johnb432 johnb432 deleted the cargo-refactor branch January 14, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-fix Release Notes: **FIXED:** kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ACE Cargo actions don't show up under special circumstances with JIP
3 participants