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

Make money and supply convoy vehicles faction appropriate #30

Open
wants to merge 20 commits into
base: patch-50
Choose a base branch
from

Conversation

jwoodruff40
Copy link

@jwoodruff40 jwoodruff40 commented Jan 9, 2025

What type of PR is this?

  1. Bug
  2. Change
  3. Enhancement
  4. Miscellaneous

What have you changed and why?

Information:

Nearly all rebel templates don't have "vehiclesCivSupply" defined and inherit from A3 vanilla. This means money and supply convoys use the boxer van which looks out of place, is too fast, and is immersion breaking for many scenarios.

This PR selects an objective vehicle for those convoys from "vehiclesCivSupply" if it is defined, then from "vehiclesCargoTrucks", and if that's not defined than from "vehiclesTrucks." To make it more obvious that this is the objective truck, a cargo container is then loaded into the truck. The cargo container is the actual objective - consequently, to complete the mission, players will need to steal or destroy the container, regardless of whether or not it's still in the objective truck it was initially loaded in.

Please specify which Issue this PR Resolves (If Applicable).

"This PR closes Official Antistasi Community #3475!"

Please verify the following.

  1. Have you loaded the mission in LAN host?
  2. Have you loaded the mission on a dedicated server?

Is further testing or are further changes required?

  1. No
  2. Yes (Please provide further detail below.)

How can the changes be tested?

Steps:


Notes:

I'm submitting this as a PR in your repo instead of in the main ultimate repo because there were a couple merge conflicts I had to sort out between your PR and my work alone, so figure it makes sense to just merge them here if you want and it'll auto reflect in the ultimate PR.

I merged my work into your PR and implemented the suggestions I left on your original PR.

Also, not sure if it's just my setup or not, but I'm getting script errors from fn_convy.sqf line 10 with occ / inv not being defined, but these shouldn't affect any convoy type except the new repair convoy.

@wersal454
Copy link
Owner

oh crap, I haven't figured out at first

I thought "Why the hell am I the reviewer?" and then I actually noticed that this is my repo and not ultimate

Copy link
Owner

@wersal454 wersal454 left a comment

Choose a reason for hiding this comment

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

the shtick about supply vehicles, is that they look like box vehicles, but without anything in them

so a couple of things to do:
1st - supply mission is supposed to be like rebel one, when you supply civ city with medical/food supply. You can check it in related stringtable entry
image

2nd - let's do it this way, add additional check, if the vehicle is from civ supply category or not, if it is, don't add cargo box to it

3rd - because of the mission description, you should probably remove ammobox and equipmentbox

4nd - since you checking for money convoy too, do the separate check for it
and maybe look at vanilla props for the "lookalike" moneybox thing, and then if vehicle is not from civsupply category, load this prop on the truck
(if prop is not defined in the logistics, add it or pin me and I'll do it)

@wersal454
Copy link
Owner

"Also, not sure if it's just my setup or not, but I'm getting script errors from fn_convy.sqf line 10 with occ / inv not being defined, but these shouldn't affect any convoy type except the new repair convoy."

this will affect armor convoy, but I'm not sure about how to do it properly, don't know if there is a better or the similarly simple way to get all the armored vehicles from the template

maybe @SilenceIsFatto will have some idea

@jwoodruff40
Copy link
Author

2nd - let's do it this way, add additional check, if the vehicle is from civ supply category or not, if it is, don't add cargo box to it

Yeah I probably should have done this. Easy fix.


1st - supply mission is supposed to be like rebel one, when you supply civ city with medical/food supply. You can check it in related stringtable entry

3rd - because of the mission description, you should probably remove ammobox and equipmentbox

I was thinking about actually adding a couple different versions of the supply mission. One would remain how it is now (delivering medical supplies). Another might be humanitarian supplies (food/water/etc). And similar. This would allow us to use more of the vanilla props without needing to fall back to generic cargo or ammo boxes.


4nd - since you checking for money convoy too, do the separate check for it
and maybe look at vanilla props for the "lookalike" moneybox thing, and then if vehicle is not from civsupply category, load this prop on the truck

Related to above response. I wasn't able to find anything other than cash / money rolls with a quick look, but I'll keep investigating. This is why I use the generic brown cargo box on anything it fits in for the time being.


(if prop is not defined in the logistics, add it or pin me and I'll do it)

I believe I know how to do this, but I'll ping you if I have issues.

@SilenceIsFatto
Copy link
Collaborator

SilenceIsFatto commented Jan 11, 2025

"Also, not sure if it's just my setup or not, but I'm getting script errors from fn_convy.sqf line 10 with occ / inv not being defined, but these shouldn't affect any convoy type except the new repair convoy."

this will affect armor convoy, but I'm not sure about how to do it properly, don't know if there is a better or the similarly simple way to get all the armored vehicles from the template

maybe @SilenceIsFatto will have some idea

occ/inv are used in the macros, they aren't variables
use Occupants/Invaders instead, see if it makes a difference

Actually look at my feedback in #428, that line might be redundant altogether

Get vehiclesCivSupply from the rebel template, not occ/inv.

Don't attempt to load cargo into vehicle if it came from
vehiclesCivSupply (they don't have cargo nodes because they're enclosed
vehicles by design).

Set the objective object that must be stolen / destroyed to either the
vehicle OR the cargo, depending on if cargo was loaded or not (see
above).

Ensure termination conditions work correctly.

Only delete the objective object in termination if it's a cargo object.
Otherwise, let A3A_fnc_VEHdespawner deal with the vehicle itself.
@jwoodruff40
Copy link
Author

@SilenceIsFatto @wersal454 I need your thoughts:

The original intent of this PR was to not use vanilla A3 vehicles for these missions when they don't make sense for the factions involved. I deliberately, if at all possible, am trying to make this work without having to update all 5 billion faction templates; however, I've hit a major snag:

When the faction config hashmaps (e.g. A3A_template_reb) are created by fn_compatibilityLoadFaction, it calls fn_loadFaction with both the default template and the actual faction template, so the hashmap is first loaded with the items defined in the faction default templates (e.g. Templates\Templates\FactionDefaults\RebelDefaults.sqf). Then, if any given key is overridden in the faction template, the value for that key in the faction config hashmap is overwritten. What this means is that the 99% of rebel templates that don't define "vehiclesCivSupply" will by default use the boxer van since it's defined in RebelDefaults. We have no way to determine if that value was deliberately omitted by the faction creator because they wanted to use the default, or if the creator simply forgot to include a value here or didn't have an appropriate, enclosed vehicle to use. And again, don't want to manually mess around with every faction template.


Consequently, I have a proposal, and am seeking your opinion on whether this would be acceptable:

"vehiclesCivSupply" is not used for anything in Antistasi at all other than these supply / money convoys.

What if we don't use this array at all anymore, and instead always use a cargo truck from the faction templates in use, with an appropriate container acting as the objective itself. My thought is that this would go hand in hand with wersal's convoy variety by war level. E.g. at lower war levels, the truck would be selected from the civilian template (if civilians are enabled in the game). This aligns with how the missions currently "feel," using a civilian vehicle. At higher war levels (or if civilians aren't enabled), a truck from the appropriate Occ or Inv faction will be used, again with a cargo container loaded.


I'm going to go ahead and run with this idea and throw something together for it, but figured y'all can ruminate on the idea while I work it out.

@jwoodruff40 jwoodruff40 marked this pull request as draft January 11, 2025 22:58
@wersal454 wersal454 marked this pull request as ready for review January 12, 2025 01:48
@wersal454
Copy link
Owner

Actually, I don't think there is a need in removing vehiclesCivSupply altogether, just do getordefault

And if I'm understanding what you are saying correctly, just remove boxer van from faction default, with the changes already made, it's not needed anymore
image
If I'm reading the code correctly, you already checking if van exists or not and if not replacing it with something else

btw, maybe when defaulting from civsupply vehicle, default to civ truck, either from rebel template or from civ template

so you won't be using army trucks, idk

instead of inheriting defaults.

Add variety of objective vehicle by war level: civilian --> militia -->
military ( + medical-specific)
@jwoodruff40
Copy link
Author

jwoodruff40 commented Jan 12, 2025

Let me clarify -

  1. I don't intend to remove vehiclesCivSupply from the faction templates that already have it (again, I don't want to mess with the templates). I just intend to stop using it.

  2. The issue with getOrDefault that I'm currently using, is that I realized factions will always have vehiclesCivSupply. Either it will be defined in the faction template, or it will be inherited from the RebelDefaults template. Therefore, the default condition (empty array) will never be used. You can check this by taking a look at A3A_faction_reb get "vehiclesCivSupply"; while using a mod faction that doesn't define vehiclesCivSupply.

  3. I don't want to remove the vehiclesCivSupply definition from RebelDefaults or any other template unless / until we decide that this is a workable solution.

  4. default to civ truck, either from rebel template or from civ template

This is the plan, except it's tiered. Starts with civ only --> civ + militia --> militia + mil --> mil only


Additional notes:

  1. Using this solution, it's still possible to select a vehicle that cannot load any cargo. This is going to be far more prevalent with civilian vehicles I believe, which is fine. Those that are able to load cargo will load it, those that aren't will not. For example, the vanilla boxer van I keep complaining about is part of vehiclesCivIndustrial for the vanilla templates. Therefore, it may be selected at low war levels. If it is, the code will not attempt to load cargo into it.

  2. Military faction vehiclesMedical is used at some war levels. For many factions, this includes medical APCs. I don't personally see an issue with this, it just makes the mission somewhat more difficult, but let me know if you disagree.


Please take a look at the code I'm about to push and let me know your thoughts. I think the code explains what I'm trying to do better than I can explain in words; however, note that this code is not tested because I'm writing it on a separate machine from my gaming pc.

@wersal454
Copy link
Owner

image
this one is fine overall, though it seems as overcomplication to me

and I think this one
image
will fix the issue you are having with defaults, so you can add vehiclesCivSupply back

@SilenceIsFatto
Copy link
Collaborator

SilenceIsFatto commented Jan 12, 2025

@SilenceIsFatto @wersal454 I need your thoughts:

The original intent of this PR was to not use vanilla A3 vehicles for these missions when they don't make sense for the factions involved. I deliberately, if at all possible, am trying to make this work without having to update all 5 billion faction templates; however, I've hit a major snag:

When the faction config hashmaps (e.g. A3A_template_reb) are created by fn_compatibilityLoadFaction, it calls fn_loadFaction with both the default template and the actual faction template, so the hashmap is first loaded with the items defined in the faction default templates (e.g. Templates\Templates\FactionDefaults\RebelDefaults.sqf). Then, if any given key is overridden in the faction template, the value for that key in the faction config hashmap is overwritten. What this means is that the 99% of rebel templates that don't define "vehiclesCivSupply" will by default use the boxer van since it's defined in RebelDefaults. We have no way to determine if that value was deliberately omitted by the faction creator because they wanted to use the default, or if the creator simply forgot to include a value here or didn't have an appropriate, enclosed vehicle to use. And again, don't want to manually mess around with every faction template.

Consequently, I have a proposal, and am seeking your opinion on whether this would be acceptable:

"vehiclesCivSupply" is not used for anything in Antistasi at all other than these supply / money convoys.

What if we don't use this array at all anymore, and instead always use a cargo truck from the faction templates in use, with an appropriate container acting as the objective itself. My thought is that this would go hand in hand with wersal's convoy variety by war level. E.g. at lower war levels, the truck would be selected from the civilian template (if civilians are enabled in the game). This aligns with how the missions currently "feel," using a civilian vehicle. At higher war levels (or if civilians aren't enabled), a truck from the appropriate Occ or Inv faction will be used, again with a cargo container loaded.

I'm going to go ahead and run with this idea and throw something together for it, but figured y'all can ruminate on the idea while I work it out.

If it works fine without a seperate category, I don't see any problems

Civilian vehicles only - vehiclesCivIndustrial / vehiclesCivSupply for
money convoys, vehiclesCivMedical / vehiclesCivSupply for supply convoys
so getOrDefault works as intended in
core\addons\functions\Missions\fn_convoy
@jwoodruff40
Copy link
Author

I think you're right - it's getting overcomplicated. Let's do this -

  1. If vehiclesCivSupply is specifically defined in a faction template, we know the faction creator wanted to use that vehicle(s), so we will only use that.
  2. If it's not defined, we'll use vehiclesCivIndustrial (civilian cargo vehicles, generally) for money convoys and vehiclesCivMedical for supply convoys.
  3. If civilian faction is disabled in the game and vehiclesCivSupply is not defined by the rebel faction, only then will we use vehiclesMilitiaTrucks.

This should simplify the cases and keep the feel more in line with how antistasi currently is. We do need to entirely remove vehiclesCivSupply from RebelDefaults though, not just set it to empty array.

@wersal454
Copy link
Owner

image

image
image
and I think, if it's possible, to do a composition with empty box and money, like that
image

@jwoodruff40
Copy link
Author

Can the IDAP items be used when players don't have Laws of War? That was my hesitation to use those. I also admit I don't know anything about compositions. Otherwise, probably a lot better than my shitty retex's.

@wersal454
Copy link
Owner

well, yeah, why not? You are not carrying it or driving it, so dlc notifications stuff won't pop up in your face

as for availability for the dlc stuff, it's vanilla dlc, content from it is preinstalled and is always available

@jwoodruff40
Copy link
Author

jwoodruff40 commented Jan 13, 2025

Ok, so the models / textures are installed with the game regardless of whether you have the dlc or not? Good to know. In that case, we definitely have a lot more options for the humanitarian / medical convoys, but like you said will probably need to create a composition or something for the money convoys.

Will probably also need to change the mission description in scrt/stringtable.xml to read humanitarian supplies or something instead of medical supplies.

@wersal454
Copy link
Owner

So I asked someone about creating a composition and saving it as a classname, I guess it somewhat hard to do and unless anyone knows for sure on how to do this, it's probably not worth it

My suggestion is, let's leave money box thing as is(i.e. as a retexture(btw can you post a screenshot on how it looks,I'm busy irl right now, so I can't check it out)))
As for supplies, if you gonna add something else as an object and not your retexture, just make sure it has an entry in logistics

And I would suggest final review from @SilenceIsFatto before I merge it

@jwoodruff40
Copy link
Author

I'll be working on this more probably tomorrow. In the meantime, here's some screens.

The medical ones I think actually turned out alright. The money ones are kind of ... :puke: and I need to fix the texture on top so they actually line up

I'm not a graphic artist lol
supply crates
supply crates medical
supply crates money 2
supply crates money

@jwoodruff40
Copy link
Author

jwoodruff40 commented Jan 19, 2025

Ok I went ahead and added the humanitarian cargo items (all in the picture). Still not sure what to do about money.
humanitarian supplies

~~ I'm going to try to retex the water bottle pallet (wrapped in plastic wrap) to be a wrapped money pallet, but not sure if it will work without entirely replacing that texture because that item doesn't have hiddenSelections / hiddenSelectionTextures defined. (well actually none of these do for some reason, only the little boxes do). I think if it doesn't work, we can just retex the water bottle pallet only for money and take it out of the humanitarian convoys entirely since there's plenty of other cargo for those missions.~~

Also still need to change the mission description for humanitarian or medical (could still be medical supplies mission if a medical truck is used with no cargo).

@jwoodruff40
Copy link
Author

So.

Turns out you can't overwrite vanilla textures at all if they don't have hiddenSelections / hiddenSelectionsTextures. I couldn't find any good large containers to retexture for money convoys, only the little boxes.

money pallet

I found a sort of way to get around the composition issue by retexturing a couple small boxes as money crates, attaching them to a pallet, and using that. Works great ... except, (I think) because the money crates are attached to the pallet and the pallet is then attached to the vehicle, the money crates kind of "wobble" around when the vehicle is moving. Not a great effect.

wobbling containers

In any case, here's some screens of that. If we can't figure out a way to make them not wobble, may just need to settle for loading a tiny money crate in a large truck. (it is more realistic for the amount of money you get though...)

The humanitarian containers added earlier seem to work fine.

Other than the money crate annoyance, I'd say this is #ready-for-testing

Add money crate retex; use in money convoys (either 1 or 3 on a pallet).

Add pallet and crate to logisitcs.

Rework adding container to vehicle to try large size container first,
then small. Otherwise, objective will still be the truck itself.
@jwoodruff40 jwoodruff40 requested a review from wersal454 January 20, 2025 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants