harvestStorage bugs

[MaDDoXbr]

harvestStorage can be set in UnitDefs but it can't be read in-game, requiring the use of a customParams (we're using unitDef.customParams.maxorestorage).

Also, currently Spring.SetUnitHarvestStorage respects the unitDef's harvestStorage (max amount), but if the harvester unit is reclaiming a metal feature, that limit is not respected.

[lhog]

harvestStorage is considered deprecated, use harvestMetalStorage and harvestEnergyStorage instead.

Also, currently Spring.SetUnitHarvestStorage respects the unitDef's harvestStorage (max amount), but if the harvester unit is reclaiming a metal feature, that limit is not respected.

For this one I need a code of what you're trying to do and there the issue is. I understand the code better than the textual description.

[lhog]

@MaDDoXbr have you checked by a chance?

[MaDDoXbr]

Sorry @lhog, somehow notifications aren't getting to my email.. I just decided to check on this issue now, as you were discussing resources in the #engine discord channel.

As for the issue itself, the wording is probably poor. It's as simple as, 'setting the unitDef.harvestStorage fires no error, but reading it in game results in nil'. Probably because of the 'harvestMetalStorage' change, although I believe I tested it and it also didn't work. I'll test it again to be sure.

The second point seems to be related to something else, apparently the metalStorage is somehow being affected, it even show in the unit's infobox.. ouch, what a mess, let me dig through this again and I'll create a specific repro scenario + lua script so you can easily see it yourself in-game.

[lhog]

@MaDDoXbr any feedback?

[sprunk]

I can't reproduce the "cannot read the value ingame", works for me. But I can confirm some other issues:

• have a unit with 10 metal harvest storage and 50 energy harvest storage. Reclaim a feature with 15 metal and 30 energy content.
• initially it reclaims normally, until 33% reclaim left. At this point the unit has 10m/20e stored (metal is full!), and the feature has 5m/10e remaining.
• the unit keeps reclaiming just the energy, ignoring the metal. At some point the feature approaches 0% reclaim and 0 energy, then it disappears while still having 5 metal inside, which is wasted.
• if the feature has 0 energy, the reclaim stops when metal is full (correct behaviour).
• if the con has 0 storage of either kind it's the same as that resource being always full (doesn't reclaim features containing just that resource, completely ignores and wastes that resource for features both).

Expected behaviour: either the reclaim meter starts lowering proportionally slower and the feature is left at 5m/0e (11% reclaim), or the harvest stops already at 5m/10e.

Side note, ZK isn't the main client here but we somewhat wanted to make Puppy work via harvest. All trees in ZK contain 0.001 metal (because otherwise the engine would deproritize them on area reclaim) so a metal-only harvester would 100% waste all trees.

Other notes:

• reclaim.reclaimMethod > 1 doesn't seem to be respected, it always treats higher values as 1 (i.e. single chunk). It seems like this is broken overall for all reclaim (regardless of harvest) but I haven't checked too deeply. I don't think the multiple-chunk mode makes much sense as a modrule (if anything, it would be good per feature(def) - like in Starcraft 1 you have minerals which harvest in discrete chunks of 8 minerals) so this may be a good opportunity to remove it?
• in single-chunk mode (reclaim.reclaimMethod == 1, though since see above), buildpower is always applied, and then if all storages are full the feature is left almost-reclaimed at 0.01% - it should probably not apply any buildpower if it will be unable to finalize the feature? Harvesting is probably not designed for chunk reclaim so it's probably fine to stay as is though (maybe produce a warning if any unit has storage if the chunk reclaim modrule is on?).
• full harvest storage is completely ignored for unit reclaim, both in chunk mode (like BAR) and reverse-build mode (like ZK). The resources do end up in the harvest storage though, just they get wasted. Again, not what harvest is designed for, so probably doesn't matter as the game can take care to block this.
• reclaim.featureEnergyCostFactor > 0 modrule makes units not stop when fully capped if the feature has no energy content. I think this is because it sees a negative energy value and thinks something along the lines of current + (negative) step < max. On the other hand, it stops correctly on features with energy even with absurdly high energy cost multiplier so maybe it's just the negative value breaking things, idk. Regardless, the energy cost is actually paid from global storage, not from unit harvest storage, so shouldn't have any effect either way (no opinion on whether it should use harvest or global storage).

[MaDDoXbr]

I've just checked, there's no Spring.GetUnitHarvestMetalStorage (or "Set"). Sorry that I'm so bad to create repro scenes Ivand. I'll deem this entire harvestStore toolset useless and FUBAR and move it to customParams. It's not worth it really. Just to report the final (and terrible) related bug, when you do a regular reclaim the harvestStorage (or whatever) is used, the max harvestStorage is not respected, and the corpse gets into a "limbo" state - it is fully reclaimed but it's not removed from the game, nor can be reclaimed anymore. Doh.

[sprunk]

there's no Spring.GetUnitHarvestMetalStorage (or "Set").

local metalCurrent, metalMax, energyCurrent, energyMax = Spring.GetUnitHarvestStorage(unitID)
Spring.SetUnitHarvestStorage(unitID, metalCurrent, metalMax, energyCurrent, energyMax)

Set accepts nil not to change the value.

If you want convenience functions you can do (for example):

Spring.GetUnitHarvestEnergyStorage = function (unitID)
  local metalCurrent, metalMax, energyCurrent, energyMax = Spring.GetUnitHarvestStorage(unitID)
  return energyCurrent, energyMax
end

the corpse gets into a "limbo" state - it is fully reclaimed but it's not removed from the game, nor can be reclaimed anymore.

Can you give specific values at which this happens? Feature metal, energy and reclaimtime; unit harveststorage and buildpower? See above for my examples with values which generally work https://github.com/beyond-all-reason/spring/issues/315#issuecomment-1257128570

I tried a bit and corpses do stop getting reclaimed at some point and become unable to be reclaimed by a constructor, but that's just because the constructor is full. In my testing if the constructor is not full it removes features too eagerly even.

[lhog]

@MaDDoXbr leave at least a replay where it happens, preferably on the latest engine version.

[MaDDoXbr]

@sprunk I believe you got this from the source code? 'Coz those params are not described in the wiki anyhow, and I didn't go check the source code (trying to keep my sanity at the high level lua scripts, but obviously failing). I'm having a hard time replicating that exact behavior to send you a replay, even if it has happened some times in some of our games (most in not-so-recent BAR engine versions). I believe the "limbo corpse" is created from my usage of gadget:AllowFeatureBuildStep, to try and deliver the feature-reclaimed metal directly to the economy (the behavior I want) instead of it being stored in the harvesting unit (which should only happen when actually harvesting). This has been a source of all sorts of issues, and even with Sprung and other trying to help me with that, the "part" calculation (of featureBuildStep) has been really inconsistent in my tests. It seems that the amount of harvestStorage affects the part calculation, instead of only considering the reclaimed amount, I'm not sure. Anyways, I'll try it a bit more with the params Sprung has mentioned, but I'm not keeping my hopes high at this point.

[sprunk]

I believe you got this from the source code? 'Coz those params are not described in the wiki anyhow, and I didn't go check the source code

Wiki is sometimes outdated, sometimes incorrect, and generally sucks ass. Don't trust it. (Wiki sucking ass is a valid complaint, but orthogonal to harvest storage.)

gadget:AllowFeatureBuildStep, to try and deliver the feature-reclaimed metal directly to the economy (the behavior I want) instead of it being stored in the harvesting unit (which should only happen when actually harvesting)

When should these two things (delivering to the economy VS being stored in the harvester) happen? If you just want a harvester to harvest first and then return the metal to the economy (at a town hall like in warcraft), that's not done via AllowFeatureBuildStep (but manually via a custom command, perhaps triggered by the max storage event). Or do you have two feature types, one that should be harvested and one that shouldn't, that are supposed to both be handled by the same constructors? If yes then that's a use case that the current setup just doesn't support (perhaps there should be a feature tag that forces the feature to have different reclaim semantics regardless of the constructor).

[MaDDoXbr]

@sprunk The resource (ore chunk) is actually a neutral unit, and not a feature. This allowed me to make it targettable only by a certain weapon type, used by builders exclusively for that (as you mentioned, that's unsupported for features). HarvestStorage being used for regular metal reclaim (when the unitDef is set) was a surprise for me, and even if it makes sense I didn't want it in TAP for a number of reasons, some gameplay-related and other balance-related. Then I tried to use AllowFeatureBuildStep to circumvent this behavior, by adding to the team resources directly whatever was harvest-stored, then revert harvestStorage to the previous value. Terrible idea, it threw me into this rabbit hole. For the record, I've just removed harvestStorage-related entries completely from the game, and after a couple hours it's all working perfectly. I also found out that simply having harvestmetalstorage set in a unitDef would cause the 'limbo feature corpse' issue, every time, but it might be related to other gadgets I'm using in-game so I wouldn't bother with that. I certainly don't wanna waste not even one more second on this, I'll just pretend it never existed in the engine.

Thus, I'd recommend totally deprecating harvestmetalstorage & related, it's pretty easy to replicate in lua (some sendluauimsg and customparams here and there, but no biggie), and it's much more flexible - not to mention reliable - this way.

Thanks for the help guys.

[sprunk]

is actually a neutral unit, and not a feature. (...) targettable only by a certain weapon type

That's the problem, it only supports features and the native reclaim command. This is because keeping track of four numbers isn't really the point of it, it's replicating the reclaim behaviour for free. See below.

it's pretty easy to replicate in lua

Are you sure about the minor things and edge cases required to make it polished? Area commands, pathing to difficult-to-reach objects, handling right-click by default, ...? Even if each individual facet is fairly easy there's just a lot to do. Note that harvesting is a fairly fundamental mechanic to how resources work in most RTS games so are one of the first things a new modder would likely ask for, and the expected skill/awareness of the user could be low. As far as I am aware harvesting as-is is essentially sufficient to implement Starcraft and Red Alert style resources in the naive way (where minerals/ore are just features and workers/miners are reclaim-only constructors) and adding the "drop off to base" mechanic to TA style reclaim only requires some thought around things that contain both resources. Fancy handling of specialized constructors that can only target specific reclaim etc (which seems to be your use case) is not the use case harvesting is currently designed for. I agree it would be good if this was communicated better, but I don't think it would be good to deprecate it if the engine aims to be a general RTS one (as long as it isn't unreasonably expensive, but given the point is that it reuses 99% of regular reclaim it should never be since it's just a bunch of if's).

[MaDDoXbr]

A specific armor category makes that pretty simple to apply for weapons. As for the actual harvesting AI, so the units do the back-and-forth to drop-off points, handle edge cases, etc, that took a lot of effort indeed, you’re very right. But it’s now done and pretty solid on TAP, (talks with auto-reclaim, etc) so we have at least one working example for new modders. It could be good leaving it as-is, if you think it’s good enough (personally I don’t). If so, I can add some extra warnings to the wiki, to warn any poor souls of its shortcomings.

[lhog]

Is this one still unclear or considered broken? I'm going to close if there's no replay is provided to show the broken behavior in a reasonable amount of time.

[MaDDoXbr]

@lhog I consider harvestStorage broken, as in it's got erratic behavior and its value can't be read in one of the spaces (wg or gg, can't recall which). When I tested harvestStorageMetal the result was the same, but Sprung's results vastly differed.

I'm not using it anymore like I said, but since removing harvestStorage would harm ZK, I recommend leaving it untouched. Another specific issue can be created when more specific / replicate-able errors can be pointed.