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

Add support for items that disable other item slots #5664

Merged
merged 8 commits into from
Apr 1, 2023

Conversation

Regisle
Copy link
Member

@Regisle Regisle commented Feb 9, 2023

This adds support for disabling helm/body/rings/5th flask from things like bringer of rain (disables body)

supersedes #1317

"can't use helmets" isnt properly supported due to being on a passive which isnt fully processed by the time items are added, a substitute ("can't use helmet") was added, but must be applied to an item to work atm, may come up with a workaround to it

@Regisle Regisle mentioned this pull request Feb 9, 2023
@QuickStick123 QuickStick123 added the enhancement New feature, calculation, or mod label Feb 10, 2023
Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

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

This works, but the code is a bit messy. Using itemDisabled to create a stack of dependent items, then setting each of those slot items to nil if they're all valid seems cleaner to me. For stalemates you'd just have to check if the item you're looking at is already in the stack.

Failing that, I think size can be removed almost everywhere, just by looking at how you're using it.

Comment on lines +3525 to +3526
--["can't use helmets"] = { mod("CanNotUseHelmet", "Flag", 1, { type = "DisablesItem", slotName = "Helmet" }) }, -- this one does not work due to being on a passive?
["can't use helmet"] = { mod("CanNotUseHelmet", "Flag", 1, { type = "DisablesItem", slotName = "Helmet" }) }, -- this is to allow for custom mod without saying the other is parsed
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should include either of these lines if it's not supported. Getting it supported would be nice for the power report, though.

Comment on lines +648 to +650
if modDB:Flag(nil, "CanNotUseHelm") then
itemDisabled["Helmet"] = { disabled = true, size = 1 }
end
Copy link
Member

Choose a reason for hiding this comment

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

Similar to the other comment about helmets, this probably should just be removed.

@Regisle
Copy link
Member Author

Regisle commented Mar 31, 2023

This works, but the code is a bit messy. Using itemDisabled to create a stack of dependent items, then setting each of those slot items to nil if they're all valid seems cleaner to me. For stalemates you'd just have to check if the item you're looking at is already in the stack.

Failing that, I think size can be removed almost everywhere, just by looking at how you're using it.

cleaned up most of it, just still have the helm mods, which I havnt removed because I personaly would use it, but agree it should be properly supported in future

@Wires77
Copy link
Member

Wires77 commented Mar 31, 2023

I noodled on this some more from work and made a PoC on https://www.jdoodle.com/execute-lua-online/ Play with the causers table to make sure I didn't miss any edge cases, but it should print the numbers that should be disabled. The only "issue" I see is that it's potentially unpredictable in how it handles cycles (aka which half of the cycle is disabled).

For this PR we'd just have to build the two tables as you have in the initial loop, then change the numbers to their slots. Finally loop through trueDisabled to set the items to nil

(Also these are PoC names, so feel free to change them to better suit the items)

function prettyPrintTable(tbl, pre, outFile)
	pre = pre or ""
	local outNames = { }
	for name in pairs(tbl) do
		table.insert(outNames, name)
	end
	table.sort(outNames)
	for _, name in ipairs(outNames) do
		if type(tbl[name]) == "table" then
			prettyPrintTable(tbl[name], pre .. name .. ".", outFile)
		else
			if outFile then
				outFile:write(pre .. name .. " = " .. tostring(tbl[name]) .. "\n")
			else
				print("%s%s = %s", pre, name, tostring(tbl[name]))
			end
		end
	end
end

local disabled = {}
local causers = {}
causers[1] = 2
causers[2] = 3
causers[3] = 4
causers[4] = 1

for num, disable in pairs(causers) do
    disabled[disable] = num
end

visited = {}
trueDisabled = {}
for num, disabler in pairs(disabled) do
    if not visited[num] then
    
        -- find chain start
        curChain = { num = true }
        while disabled[num] do
            num = disabled[num]
            if curChain[num] then break end -- detect cycles
            curChain[num] = true
        end
    
        repeat
            visited[num] = true
            num = causers[num]
            if not num then break end
            visited[num] = true
            trueDisabled[num] = true
            num = causers[num]
        until(not num or visited[num])
    
    end
    
end

prettyPrintTable(trueDisabled)

Copy link
Member

@Wires77 Wires77 left a comment

Choose a reason for hiding this comment

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

Good to go, hopefully we can include Dance with Death properly some day

@Wires77 Wires77 changed the title Cant use X Add support for items that disable other item slots Apr 1, 2023
@Wires77 Wires77 merged commit a656f01 into PathOfBuildingCommunity:dev Apr 1, 2023
@Regisle Regisle deleted the CantUseXX branch April 1, 2023 10:42
Dullson pushed a commit to Dullson/PathOfBuilding that referenced this pull request Dec 6, 2023
…mmunity#5664)

* Initial Implementation of item Disablers

* fix spelling

* fix spelling

* update tooltip

* Apply suggestions from code review

Co-authored-by: Wires77 <[email protected]>

* cleanup merge issues

* Refactored algorithm for determining disabled items

---------

Co-authored-by: Wires77 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, calculation, or mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants