#3959 Refactored stockpile limits into unit custom params #3994

ryanbennitt · 2024-12-04T12:26:11Z

Work done

Refactored stockpile limits into each unit's custom params section instead.

Some values remain as cannot find these unit defs here, are they stored elsewhere or are they vestigial and can be removed?

Addresses Issue(s)

Move default stockpile limits to customparams #3959
#3959

Setup

Just a regular game, some units changed require evolved commanders, legion, raptors

Test steps

Construct any unit containing a limited stockpile of e.g. missiles, confirm that it stops building when its stockpile limit is reached

…arams

sprunk · 2024-12-04T22:29:29Z

Looks good. Decoms exist and are defined here:

ryanbennitt · 2024-12-04T23:01:02Z

Refactored these too

luarules/gadgets/unit_stockpile_limit.lua

…All-Reason into 3959-refactor-stockpile-limits

WatchTheFort

Looking at this again, the stockpile limit should be on the weapon def, not the unit def, as that is what it applies to.

ryanbennitt · 2024-12-12T20:39:37Z

Looking at this again, the stockpile limit should be on the weapon def, not the unit def, as that is what it applies to.

That might explain why I've been having trouble testing these changes...

sprunk · 2024-12-13T19:00:12Z

Caveat beyond-all-reason/spring#1817 (but it's still good to apply it to weapons)

ryanbennitt · 2024-12-14T12:40:39Z

OK so I moved ['legcom'] = 2, into legcom.lua but none of the weapons are marked as stockpile=true, so which one does it apply to? Is this an error?

ryanbennitt · 2024-12-14T12:47:30Z

Also similarly armcomlvl2 has ['armcomlvl2'] = 3, but none of its weapons are marked stockpile=true. armcomlvl3 has a backlauncher with stockpile=true but armcomlvl2 does not have this weapon, so remove its stockpile limit?

…All-Reason into 3959-refactor-stockpile-limits-4

ryanbennitt · 2024-12-18T23:10:48Z

Looking at this again, the stockpile limit should be on the weapon def, not the unit def, as that is what it applies to.

Refactored and commented out the inconsistencies e.g. stockpile limits on non-stockpile weapons...

ryanbennitt · 2024-12-18T23:12:33Z

units/Legion/Defenses/legstarfall.lua

@@ -136,6 +136,9 @@ return {
 				weapontimer = 14,
 				weapontype = "Cannon",
 				weaponvelocity = 900,
+				--customparams = {
+				--	stockpilelimit = 1,
+				--},


Stockpile limit was defined but this weapon does not have stockpile configuration

ryanbennitt · 2024-12-18T23:13:26Z

units/Legion/Legion EvoCom/legcomlvl3.lua

@@ -371,6 +371,9 @@ return {
 				weapontimer = 5,
 				weapontype = "MissileLauncher",
 				weaponvelocity = 650,
+				customparams = {
+					stockpilelimit = 3,
+				},


Unused weapon but has stockpile configuration so should have limit, or could be deleted entirely

ryanbennitt · 2024-12-18T23:13:32Z

units/Legion/Legion EvoCom/legcomlvl4.lua

@@ -363,6 +363,9 @@ return {
 				weapontimer = 5,
 				weapontype = "MissileLauncher",
 				weaponvelocity = 650,
+				customparams = {
+					stockpilelimit = 3,
+				},


Unused weapon but has stockpile configuration so should have limit, or could be deleted entirely

It should be safe to remove the napalmmissile from the legion comms, it probably won't be needed in the future and wouldn't be difficult to remake.

ryanbennitt · 2024-12-18T23:13:55Z

units/Legion/Legion EvoCom/legevodecom.lua

@@ -13,6 +13,8 @@ unitsTable['legdecomlvl3'].decoyfor = "legcomlvl3"
 unitsTable['legdecomlvl3'].customparams.decoyfor = "legcomlvl3"
 unitsTable['legdecomlvl3'].health = math.ceil(unitsTable['legdecomlvl3'].health*0.5)
 unitsTable['legdecomlvl3'].weapondefs.disintegrator.damage.default = 40
+--unitsTable['legdecomlvl3'].weapondefs.napalmmissile.customparams.stockpilelimit = 1


Unused weapon but has stockpile configuration so should have limit, or could be deleted entirely

ryanbennitt · 2024-12-18T23:14:50Z

units/Legion/legcom.lua

@@ -103,6 +103,7 @@ return {
 			normaltex = "unittextures/leg_normal.dds",
 			paralyzemultiplier = 0.025,
 			subfolder = "",
+			--stockpilelimit = 2,


Stockpile limit defined but no stockpile configuration on weapon

ryanbennitt · 2024-12-18T23:15:05Z

units/other/evocom/armcomlvl2.lua

@@ -124,6 +124,7 @@ return {
 			minimum_respawn_stun = 5,
 			distance_stun_multiplier = 1,
 			fall_damage_multiplier = 5,--this ensures commander dies when it hits the ground so effigies can trigger respawn.
+			--stockpilelimit = 3,


Stockpile limit defined but no stockpile configuration on weapon

@SethDGamre Is this supposed to be a stockpiling weapon?

WatchTheFort

@Zecrus2 Can you please review the Legion units that ryanbennitt commented on?

WatchTheFort · 2024-12-19T00:12:01Z

units/other/evocom/armcomlvl2.lua

@@ -124,6 +124,7 @@ return {
 			minimum_respawn_stun = 5,
 			distance_stun_multiplier = 1,
 			fall_damage_multiplier = 5,--this ensures commander dies when it hits the ground so effigies can trigger respawn.
+			--stockpilelimit = 3,


@SethDGamre Is this supposed to be a stockpiling weapon?

WatchTheFort · 2024-12-19T00:13:17Z

luarules/gadgets/unit_stockpile_limit.lua

@@ -28,14 +28,19 @@ if gadgetHandler:IsSyncedCode() then
 	local GiveOrderToUnit	= Spring.GiveOrderToUnit

 	local canStockpile = {}
+	Spring.Log(gadget:GetInfo().name, LOG.WARNING, "Check all stockpiles "..#UnitDefs)


This is not a warning, this should be LOG.INFO, but should really just be removed, it adds noise to the log.

WatchTheFort · 2024-12-19T00:13:41Z

luarules/gadgets/unit_stockpile_limit.lua

+			for i = 1, #ud.weapons do
+				local weaponDef = WeaponDefs[ud.weapons[i].weaponDef]
+				if weaponDef.stockpile and weaponDef.customParams and weaponDef.customParams.stockpilelimit then
+					Spring.Log(gadget:GetInfo().name, LOG.INFO, "Stockpile "..ud.name.." - "..weaponDef.name.." limit "..weaponDef.customParams.stockpilelimit)


This logging is not needed, it adds a lot of noise to the log.

WatchTheFort · 2024-12-19T00:16:53Z

luarules/gadgets/unit_stockpile_limit.lua

+			for i = 1, #ud.weapons do
+				local weaponDef = WeaponDefs[ud.weapons[i].weaponDef]


Isn't weaponDef already the weapon def?

Suggested change

for i = 1, #ud.weapons do

local weaponDef = WeaponDefs[ud.weapons[i].weaponDef]

for _, weaponDef in ipairs(ud.weapons) do

IIRC it's just the ID.

weaponDef.weaponDef is the weapon def ID. From the wiki example, it looks like weapons table is an array where each value is the weapon def.
https://springrts.com/wiki/Lua_UnitDefs

WatchTheFort · 2024-12-19T00:19:07Z

luarules/gadgets/unit_stockpile_limit.lua

+				local weaponDef = WeaponDefs[ud.weapons[i].weaponDef]
+				if weaponDef.stockpile and weaponDef.customParams and weaponDef.customParams.stockpilelimit then
+					Spring.Log(gadget:GetInfo().name, LOG.INFO, "Stockpile "..ud.name.." - "..weaponDef.name.." limit "..weaponDef.customParams.stockpilelimit)
+					isStockpilingUnit[udid] = tonumber(weaponDef.customParams.stockpilelimit)


Table name should represent what it contains. Shouldn't the table be indexed by weapon def ID, not unit def ID?

Suggested change

isStockpilingUnit[udid] = tonumber(weaponDef.customParams.stockpilelimit)

weaponStockpileLimits[weaponDef.weaponDef] = tonumber(weaponDef.customParams.stockpilelimit)

I have no idea why the engine calls the weapon def ID weaponDef instead of weaponDefID....

beyond-all-reason#3959 Refactored stockpile limits into unit custom p…

614b93d

…arams

beyond-all-reason#3959 Refactored evodecom stockpile limits

dbae945

WatchTheFort requested changes Dec 4, 2024

View reviewed changes

luarules/gadgets/unit_stockpile_limit.lua Outdated Show resolved Hide resolved

luarules/gadgets/unit_stockpile_limit.lua Show resolved Hide resolved

83N170 added 2 commits December 5, 2024 08:28

3959 Remove code populating scavengers stockpile limits

ac31e13

Merge branch 'master' of https://github.com/beyond-all-reason/Beyond-…

2d31d02

…All-Reason into 3959-refactor-stockpile-limits

WatchTheFort approved these changes Dec 12, 2024

View reviewed changes

WatchTheFort requested changes Dec 12, 2024

View reviewed changes

83N170 added 2 commits December 18, 2024 23:01

Merge branch 'master' of https://github.com/beyond-all-reason/Beyond-…

edc1d8a

…All-Reason into 3959-refactor-stockpile-limits-4

beyond-all-reason#3959 Refactor stockpile limits into weapon defs

b1fe0f7

ryanbennitt commented Dec 18, 2024

View reviewed changes

WatchTheFort requested changes Dec 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#3959 Refactored stockpile limits into unit custom params #3994

#3959 Refactored stockpile limits into unit custom params #3994

ryanbennitt commented Dec 4, 2024

sprunk commented Dec 4, 2024

ryanbennitt commented Dec 4, 2024

WatchTheFort left a comment

ryanbennitt commented Dec 12, 2024

sprunk commented Dec 13, 2024

ryanbennitt commented Dec 14, 2024

ryanbennitt commented Dec 14, 2024

ryanbennitt commented Dec 18, 2024

ryanbennitt Dec 18, 2024

ryanbennitt Dec 18, 2024

ryanbennitt Dec 18, 2024

Zecrus2 Dec 19, 2024

ryanbennitt Dec 18, 2024

ryanbennitt Dec 18, 2024

ryanbennitt Dec 18, 2024

WatchTheFort Dec 19, 2024

WatchTheFort left a comment

WatchTheFort Dec 19, 2024

WatchTheFort Dec 19, 2024

WatchTheFort Dec 19, 2024

WatchTheFort Dec 19, 2024

sprunk Dec 19, 2024

WatchTheFort Dec 19, 2024

WatchTheFort Dec 19, 2024

		for i = 1, #ud.weapons do
		local weaponDef = WeaponDefs[ud.weapons[i].weaponDef]

	for i = 1, #ud.weapons do
	local weaponDef = WeaponDefs[ud.weapons[i].weaponDef]
	for _, weaponDef in ipairs(ud.weapons) do

	isStockpilingUnit[udid] = tonumber(weaponDef.customParams.stockpilelimit)
	weaponStockpileLimits[weaponDef.weaponDef] = tonumber(weaponDef.customParams.stockpilelimit)

#3959 Refactored stockpile limits into unit custom params #3994

Are you sure you want to change the base?

#3959 Refactored stockpile limits into unit custom params #3994

Conversation

ryanbennitt commented Dec 4, 2024

Work done

Addresses Issue(s)

Setup

Test steps

sprunk commented Dec 4, 2024

ryanbennitt commented Dec 4, 2024

WatchTheFort left a comment

Choose a reason for hiding this comment

ryanbennitt commented Dec 12, 2024

sprunk commented Dec 13, 2024

ryanbennitt commented Dec 14, 2024

ryanbennitt commented Dec 14, 2024

ryanbennitt commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WatchTheFort left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment