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 eoc effect and condition, that searches weighed amount of items in your inventory #75631

Merged
merged 3 commits into from
Aug 20, 2024

Conversation

GuardianDll
Copy link
Member

@GuardianDll GuardianDll commented Aug 12, 2024

Summary

None

Purpose of change

Fix #74726
Fix #71215

Describe the solution

The idea is to have the condition like this:

    "condition": {
      "u_has_items_sum": [
        { "item": "blanket", "amount": 10 },
        { "item": "blanket_fur", "amount": 20 },
        { "item": "electric_blanket", "amount": 15 }
      ]
    },

and under the hood the game checks how much of each item user has, as percentage, so, for example, having 5 blanket and 10 blanket_fur would both count as 50%, which would result in 100% total and condition returning "true"

Effect does the same thing, but instead of checking items, it just consumes them

Testing

image
image
image

save file used to test thing
Mainsave-trimmed.tar.gz

@github-actions github-actions bot added NPC / Factions NPCs, AI, Speech, Factions, Ownership [C++] Changes (can be) made in C++. Previously named `Code` EOC: Effects On Condition Anything concerning Effects On Condition <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Aug 12, 2024
@PatrikLundell
Copy link
Contributor

So, if I understand it correctly, the code consumes items in the listed order until the percentage reaches 100 (or 1, technically).
Thus, if you had a list of:
item a, count 1000,
item b, count 2,
item c, count 1000

returning with
999 a, 2 b, 1000 c
would consume
999 a and 2 b
rather than 2b or 1000 c.

This is an extreme example, of course, but I think it illustrates the point that the order really matters, and the "conversion ratios" between items do as well.

I don't know if there is any reasonable solution to the general problem, but I'd suggest consumption of everything from a single stack should be the preferred option, i.e. if any single category provides everything required, that category should be selected. This would require the logic to change a little by having the tallying of items done on a per item basis (and you can then use the count at that step, rather than percentages). Once you've tallied what you've got available you can then use logic to decide what to consume (which I suggest would include selection of a completed single entry, if available as the preferred alternative). This would also allow for further refinement of the selection logic without affecting the tallying if desired at a later time.
It can be noted that this is essentially the logic used by crafting, i.e. first there is a pass where all possible components are tallied, then there's a selection stage where the player selects which options to consume via the UI, and lastly there's a consumption step that consumes what's been selected.

I can't see any general recommendation for how to select the order of entries in such a selection (for documentation purposes). On the one hand, you'd want the heavier entries (the ones with the lowest counts) first to reduce over consumption, but on the other hand, you'd also want things to divide nicely so you end up with exactly 100% rather than overshooting.

@GuardianDll
Copy link
Member Author

What you say is something i am aware of, but i thought i could solve it by sorting the list (either in json, shoving the most heavy stuff first, or, better, in the code, by sorting a vector from smallest count to biggest one)
Additionaly, i suspect it won't be a problem 99% of the time, most use cases would probably be limited by having all items have equal count

I agree having an ability to pick which exactly items you want to give would be cool, but i don't feel myself confident to implement it

@PatrikLundell
Copy link
Contributor

The problem with having the code determine the order is that it removes the ability of the JSON writer to select other orders deemed more suitable to their particular situation. However, I agree that a code choice removes the risk of uninformed JSON writers inadvertently making bad orders.

@github-actions github-actions bot removed astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Aug 13, 2024
@GuardianDll GuardianDll force-pushed the item_sum branch 2 times, most recently from c010687 to 9aff6b7 Compare August 13, 2024 21:41
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [Markdown] Markdown issues and PRs labels Aug 13, 2024
@github-actions github-actions bot added the [JSON] Changes (can be) made in JSON label Aug 13, 2024
@GuardianDll
Copy link
Member Author

It's online and working, but i need to test it first. Anyone happen to have tacoma save file?

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Aug 13, 2024
@GuardianDll GuardianDll marked this pull request as ready for review August 14, 2024 06:57
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Aug 14, 2024
@Maleclypse
Copy link
Member

It's online and working, but i need to test it first. Anyone happen to have tacoma save file?

If you made one can you load it up in an issue labeled "Debug Save Files"? I'll mark it so it won't stale.

@GuardianDll
Copy link
Member Author

GuardianDll commented Aug 15, 2024

Where i should post it, again? I think i still have it around, albeit it's Karol save frok and 2022, so it has few errors popping at the start
UPD: posted in the body of PR

@dseguin dseguin merged commit 36d1042 into CleverRaven:master Aug 20, 2024
28 checks passed
@GuardianDll GuardianDll deleted the item_sum branch August 20, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. EOC: Effects On Condition Anything concerning Effects On Condition [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs NPC / Factions NPCs, AI, Speech, Factions, Ownership
Projects
None yet
4 participants