-
Notifications
You must be signed in to change notification settings - Fork 319
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
Resource Sharing Tax Mod: Prevent upgrading allied mex/geos. No longer disables unit sharing completely, and can buy t2 con through Unit Market #4010
base: master
Are you sure you want to change the base?
Conversation
This seems counter intuitive |
if existsNonOwnedMex(builderTeam, x, y, z) then | ||
return false | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not using the unitID it returns, you could instead have it return a bool and return that instead.
instead of returning false or running the is geo check before returning true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change made bdeb49a
local t2cons = {"armack", "armacv", "armaca", "armacsub", "corack", "coracv", "coraca", "coracsub"} | ||
function contains(table, value) | ||
for _, v in pairs(table) do | ||
if v == value then return true end | ||
end | ||
return false | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a table of unitDefIDs instead, keyed using the id returns true. rather than getting the unit name and going through the entire table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with @robertthepie, also note table.contains already exists and this is basically reimplementing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh it's defined in the project. I'm not the most familiar with lua, couldn't believe that such a basic function didn't exist already. thanks for the heads up sautron.
changes made 8d84a20
if not unitID or unitsForSale[unitID] == nil or unitsForSale[unitID] == 0 then return end | ||
local unitDefID = spGetUnitDefID(unitID) | ||
if not unitDefID then return end | ||
local unitDef = UnitDefs[unitDefID] | ||
if not unitDef then return end | ||
|
||
|
||
-- Only allow selling t2 cons when unit sharing is disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Limiting it to only selling t2 seems counterintuitive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the unit market to implement an exception -- Since that is the game flow players are used to. In general, buying 'anything' is a form of boosting, since you save the E cost, BP, and time. Allowing buy/sell of t1 cons lets you build safe eco anywhere, and you can easily "let" allies cap mexes arbitrarily at the start of the game. For example a front player could buy 2 cons from the backline, use them to cap the backliner secondary mexes and make winds, effectively boosting themselves.
By default tax unit sharing will only disable sharing of economy/labs. You are free to share combat units or other units freely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, this is a competitive modoption for a small subset of players. The ruleset has to cover everything to achieve the desired effect (also a similiar ruleset hasn't been tested before). I'm open to ideas to make the option selection more explanatory.
For ruleset changes maybe it would be better to discuss in https://discord.com/channels/549281623154229250/1253060706039890000
Tax Resource Sharing can be explained as: "You can't share eco/production, can't assist allied labs/blueprints, and resource sharing is taxed. You can buy/sell t2 at normal price via unit Market".
…ad of unused unitDefID
…t need to implement stub unitTypeAllowedToBeShared() in disable_unit_sharing. Clarified the modoption descriptions.
Added finer options for disable unit sharing, modoption features complete fd5915c Disable unit sharing testNo econ or production sharing: The unit market usage could be improved -- prevent non-valid units (non t2 cons) from being put on sale to reduce clutter/confusion. |
-- List storages manually (some units or buildings may provide e or m storage but they are not primarily econ) | ||
local storageNames = { | ||
"armestor", "corestor", "legestor", "armuwes", "coruwes", "leguwes", "armuwadves", "coruwadves", "leguwadves", | ||
"armmstor", "cormstor", "legmstor", "armuwms", "coruwms", "leguwms", "armuwadvms", "coruwadvms", "leguwadvms", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps a "dedicated storage" customparam? or some sort of "if stores more than it costs" heuristic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A customparam like "dedicatedEnergyStorage : true" is fine? Might be a useful classification for other gadgets too. A heuristic seems hackey and harder to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented it then realized unitgroup "metal" and "energy" already exist.
Changed to use customparam unitgroup. b46f7c8
…ssisting allied construction
…ce Sharing. Do place unit on sale checks properly, and clean up related UnitMarket code
…ake was not failproof anyways as winds/tidal/solar did not have that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally not a fan of the t2 cons limit, and this is starting to feel like spaghetti 🍝
if(capture) then | ||
return true | ||
end | ||
return false | ||
return allowedToBeShared(unitDefID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this come from???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh what the heck, that should be unitTypeAllowedToBeShared()
local t2conNames = {"armack", "armacv", "armaca", "armacsub", "corack", "coracv", "coraca", "coracsub", "legack", "legacv", "legaca", "legacsub"} | ||
local isT2Con = {} | ||
for _, name in ipairs(t2conNames) do | ||
if UnitDefNames[name] then | ||
isT2Con[UnitDefNames[name].id] = true | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set t2conNames to nil after it has been used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this by convention or for something like garbage collection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't take a lot of memory, but it frees it up i believe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. The var isn't used anywhere else afterwards so the Lua interpreter knows on its own to free the memory. I think the reason some wupgets do this is cargo cult.
… unused variables
Work done
Buying t2 cons:
Allied Construction:
Unit Sharing
Typical Usage
Test steps (partial)
When Unit Market enabled together with Disable Unit Sharing or Tax Resource Sharing
....
Issues
I'm getting frequent luaui crashes when using Unit Market (Build Menu and Buy signs ($) goes invisible until /luaui reload) in single player.
Discord thread: https://discord.com/channels/549281623154229250/1253060706039890000