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

Modoptions for resource sharing tax, no sharing units, and no allied build assist #2883

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

Conversation

Rimilel
Copy link

@Rimilel Rimilel commented Apr 24, 2024

Work done

Added modoptions for taxing resource sharing and disabling allied build assist (aka boosting), plus one that disables unit sharing.

Addresses Issue(s)

Game-balance wise, restricts / attempts to balance boosting by limiting it to tactical use. In high-level lobbies, on most maps, especially land+naval and role maps, boosting is an extremely powerful mechanic which favours pooling resources on the most impactful unit at any stage of the game, which comes at the expense of individual unit choice/economy management.

This change is intended to be a option for players, not a base feature.

Setup

Enable the 3 modoptions under "Restrictions".

Test steps

Enable modoptions under "Restrictions" and Open a skirmish game with an allied Inactive AI.

Set starting metal and energy to 0. Enable cheats and use /atm. Now try giving 1000 metal to your AI ally. They should only recieve a reduced amount (600 metal for 0.4 tax)

Confirm assisting an allied blueprint doesn't work. Confirm assisting an allied lab doesn't work.
Confirm cannot guard-assist allies' builders and units that can reclaim (e.g. webbers, spybots)
Confirm can reclaim enemy and own units, but not allies' units.
Confirm cannot share units to allies.

Notes

Managing Resource overflow wasn't possible here and would require a change to the engine code.

Screenshots:

spring_fHyeoIaHxb

First time contributing and doing a pull request. Let me know if there's anything amiss.

Copy link
Contributor

@robertthepie robertthepie left a comment

Choose a reason for hiding this comment

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

Apart from the minor thing with checking for the modoption every time instead of once to turn the gadgets off, and this pr sometimes using spaces instead of tabs?
looks good

Comment on lines 31 to 33
if(not Spring.GetModOptions().disable_assist_ally_construction) then
return true
end
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than having this always be active and checking this each time a command is issued, would it not be better to have it check for the modoption just after the is synced call and returning false if not enabled, as such not loading the gadget when the modoption isn't on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah it's s good idea to do this. Just cache whether modoptions is enabled once and check that instead of calling GetModOptions every time. Also if its disabled, just quit gadget on init right away. For example:

local disableAssist = Spring.GetModOptions().disable_assist_ally_construction
function gadget:Initialize()
    if not disableAssist or disableAssist ~= true then
        gadgetHandler:RemoveGadget(self)
    end
end
-- now you can check "disableAssist" later in the code if its true you do whatever you needed to do when its on, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to do anything fancy like that, if a gadget returns false it doesn't get loaded, just a duplicate of the is synced check you have on line 16 to 18, except instead of checking if it is not in synced space check if the modoption is not on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, yeah, I got used to multi-choice options.

Copy link
Author

@Rimilel Rimilel Apr 25, 2024

Choose a reason for hiding this comment

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

fixed checking modoption enabled only once and spaces

899e0ae
f9ee471

return true
end

-- Disallow guard commands onto other ally's units. (Optional todo: disallow guards on allied build-capable units and labs only)
Copy link
Contributor

@TomFyuri TomFyuri Apr 24, 2024

Choose a reason for hiding this comment

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

Probably something like this. I don't know why spies have build power though. Minelayer though its obvious, it builds mines, but it can't rez.

local spyDefs = {UnitDefNames.armspy.id, UnitDefNames.corspy.id}
local function isBuilderType(unitDefID)
	local unitDef = UnitDefs[unitDefID]
	return (unitDef.isBuilder and not unitDef.isFactory and (unitDef.buildOptions and unitDef.canReclaim and not unitDef.canResurrect) and (spyDefs[1] ~= unitDefID) and (spyDefs[2] ~= unitDefID))
end

Copy link
Author

@Rimilel Rimilel Apr 25, 2024

Choose a reason for hiding this comment

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

Settled with using this, thanks

if #targetUnitDef.buildOptions > 0 or targetUnitDef.canAssist then
	return false
end

@Rimilel
Copy link
Author

Rimilel commented Apr 25, 2024

Fixed issues and finished making other changes.

Copy link
Contributor

@robertthepie robertthepie left a comment

Choose a reason for hiding this comment

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

looks good to me.

@Rimilel
Copy link
Author

Rimilel commented Apr 29, 2024

YT video showing mod changes
-resource share taxed
-can't assist allied construction and lab
-can't guard allied builder and reclaimer
-can't share units

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.

None yet

4 participants