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

Loot in Scenario Summary only counts coins #455

Closed
tttopuz opened this issue Nov 29, 2023 · 7 comments
Closed

Loot in Scenario Summary only counts coins #455

tttopuz opened this issue Nov 29, 2023 · 7 comments
Labels
bug Something isn't working as expected spoiler This containing spoiler

Comments

@tttopuz
Copy link

tttopuz commented Nov 29, 2023

Describe the bug

At the end of a scenario, in the scenario summary window, the row Loot only counts coins, whereas it should count the number of loot tokens collected.
image

In this example, it should display Loot 1 for the Geminate.

Game Enviroment

To Reproduce

Expected behavior

Gloomhaven Secretariat Version

Used Browser

OS

Windows

Additional

No response

@tttopuz tttopuz added the bug Something isn't working as expected label Nov 29, 2023
@Jaerin
Copy link
Contributor

Jaerin commented Dec 2, 2023

It looks like character.loot was being mistaken for what should be character.gold I think. It looks like instead of just incrementing character.loot it is trying to calculate the value of the items and adding it, but only gold has a value.

applyLoot(loot: Loot, character: Character, index: number): ItemData | undefined {
let result: ItemData | undefined = undefined;
character.lootCards = character.lootCards || [];
if (loot.type == LootType.money || loot.type == LootType.special1 || loot.type == LootType.special2) {
character.loot += this.getValue(loot);
} else if (loot.type == LootType.random_item && this.game.scenario && this.game.party.campaignMode && settingsManager.settings.characterItems && settingsManager.settings.applyLootRandomItem) {
result = gameManager.itemManager.drawRandomItem(this.game.scenario.edition);
if (!result) {
character.loot += 3;
}
}

This seems to be treating character.loot as some kind of a gold value of all the items that have been collected instead of just a count of loot cards. Unless Loot is supposed to be some kind of summary gold value of loot gained but then there should be another loot card count.

@Lurkars
Copy link
Owner

Lurkars commented Dec 4, 2023

Yes, this came from Gloomhaven table, where I have the loot row, to show how much coins looted, and below the gold for counting that to gold.
image
But I think it totally right, that this should count all loot tokens collected, not the gold cards. The big difference here between Gold and other resources is still the gold conversion depending on level. So I am now unsure how to show that number. In current state, this is the loot column.

@Lurkars Lurkars added in progress Currently working on this spoiler This containing spoiler pending Additional information/feedback requested labels Dec 4, 2023
@Jaerin
Copy link
Contributor

Jaerin commented Dec 4, 2023

I'm not sure there is a time when you'd need a conversation of items to gold value at least in this situation. The question I have though is in a smaller group where a loot card counts as more than one material does that count as multiple loot cards with regards to personal quests and the like? I could see this being a reason why you'd need to calculate the "value" of the loot card, but I'm not sure on the ruling on that. That was the only reason I could come up with why you'd need to calculate the value of anything other than coins.

@tttopuz
Copy link
Author

tttopuz commented Dec 5, 2023

In my opinion, the number of loot tokens collected should be displayed here. There are for example some battle goals where you need to compare the number of collected loot tokens between players. Actual coins collected is much less important in Frosthaven.

@Lurkars
Copy link
Owner

Lurkars commented Dec 5, 2023

Yes, I also think that showing just the total number of collected tokens (e.g. collected loot cards) is here a way better information. In Gloomhaven it's also the number of loot tokens, so showing the number of coins here is just confusing. So this will change in next release. For now I will just skip the info of collected coins. If this is ever required somehow, I can rethink about where to show it.

@Lurkars Lurkars removed the pending Additional information/feedback requested label Dec 5, 2023
Lurkars added a commit that referenced this issue Dec 5, 2023
…ix scenario data, fix item rewards
@Lurkars
Copy link
Owner

Lurkars commented Dec 5, 2023

Add in v0.83.4. Happy about feedback and if issue is solved with this. Thank.

@Lurkars Lurkars assigned tttopuz and unassigned Lurkars Dec 5, 2023
@Lurkars Lurkars added to test Should be fixed, but needs proper testing and removed in progress Currently working on this labels Dec 5, 2023
@tttopuz
Copy link
Author

tttopuz commented Dec 6, 2023

Top row now displays number of loot tokens. Thanks!

@Lurkars Lurkars removed the to test Should be fixed, but needs proper testing label Dec 6, 2023
@Lurkars Lurkars closed this as completed Dec 6, 2023
rdoll pushed a commit to rdoll/gloomhavensecretariat that referenced this issue Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected spoiler This containing spoiler
Projects
None yet
Development

No branches or pull requests

3 participants