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 'done' command to treasury. #2246

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

Imisnew2
Copy link
Contributor

Once the LuaCore changes are pushed to add the 'lots' table, this PR speculatively adds the 'done' command.

The 'done' command passes on items that have not yet been either lotted or passed by the player.

@RubenatorX
Copy link
Collaborator

RubenatorX commented Nov 29, 2022

While this technically enables the "done" functionality, I was rather hoping we'd also take the time to fix lotall etc to not try lotting on things that have been passed, or lotting on things you have already lotted on, or passing on things you have already passed on as well now that we have that data to back up those decisions.

This is how I sort of proposed it:
image

Do note that my example code is not well tested and I'm realizing that I was only checking p0 and not the whole alliance so... more testing required.

@RubenatorX
Copy link
Collaborator

RubenatorX commented Nov 29, 2022

Also, looking at the commit more closely, I believe you are only passing the items that no one has lotted on, not the items that you, the player have not lotted on, so this commit should not be accepted as is.

@RubenatorX
Copy link
Collaborator

Oh and... we should also probably check that this LuaCore change doesn't implode when using the alternate treasure pool used in like.. einherjar

@Imisnew2
Copy link
Contributor Author

Also, looking at the commit more closely, I believe you are only passing the items that no one has lotted on, not the items that you, the player have not lotted on, so this commit should not be accepted as is.

Since the LuaCore changes are not yet out, it's not clear what this data actually represents. Is it the lot of some random player? Is it the lot of the current player? Who knows! I'll update the PR when I have access to the LuaCore that adds this information.

@Imisnew2
Copy link
Contributor Author

I'm not looking to refactor the entire addon. I've just added the 'done' command and fixed up the 'lotall' and 'passall' commands to not re-pass/lot things if already passed/lotted. And made a minor fix that Arcon suggested, to use get_items('treasure') instead of get_items().treasure.

@Imisnew2
Copy link
Contributor Author

Pending any further discussion, I don't plan on making any more changes to the MR. If something got missed that you wanted @RubenatorX , let me know.

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

3 participants