search

jopeek / fvtt-loot-sheet-npc-5e

FVTT - Loot Sheet NPC 5E
MIT License
44 stars 86 forks source link

[v9] Cannot create permissions or buy from Loot Sheet NPCs #207

Closed germandrewboat closed 2 years ago

germandrewboat commented 2 years ago

Describe the bug Cannot create permissions or buy from Loot Sheet NPCs

To Reproduce Steps to reproduce the behavior:

  1. Go to any created "shopkeeper"
  2. Click on permissions or any item to buy as a player user
  3. Error in console

Expected behavior Should be able to one click swap between owner, limited, observer permissions as usual, and player should have 1 click buy and transference to character sheet.

Screenshots lootsheet_ts

Desktop (please complete the following information):

DanielBoettner commented 2 years ago

Foundry v9 build 238

The module is not yet updated to work with FoundryVTT >= 0.9

We will try to get this done as soon as possible.

germandrewboat commented 2 years ago

Foundry v9 build 238

The module is not yet updated to work with FoundryVTT >= 0.9

We will try to get this done as soon as possible.

I appreciate the reply! If you need any more info let me know.

I've tagged ChalkOne on the FoundryVTT #module-troubleshooting discord channel with another screenshot showing a UI formatting bug as well in v9.

jopeek commented 2 years ago

The UI bug might be related to a conflict? Lootsheet displays fine for me in v9 without any other modules enabled. Perhaps you could verify that?

On Dec 22, 2021, at 10:10 AM, germandrewboat @.***> wrote:

 Foundry v9 build 238

The module is not yet updated to work with FoundryVTT >= 0.9

We will try to get this done as soon as possible.

I appreciate the reply! If you need any more info let me know.

I've tagged ChalkOne on the FoundryVTT #module-troubleshooting discord channel with another screenshot showing a UI formatting bug as well in v9.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

germandrewboat commented 2 years ago

If I get you a copy of my active modules list, do you think it would give you an idea on how to narrow it down? I'm just currently at work right now.

edit: I'm attaching it here. I deleted all the compendiums I could see.

edit by @DanielBoettner I removed the list as it was not needed anymore

On Wed, Dec 22, 2021, 1:17 PM Jan Ole Peek @.***> wrote:

The UI bug might be related to a conflict? Lootsheet displays fine for me in v9 without any other modules enabled. Perhaps you could verify that?

On Dec 22, 2021, at 10:10 AM, germandrewboat @.***> wrote:

 Foundry v9 build 238

The module is not yet updated to work with FoundryVTT >= 0.9

We will try to get this done as soon as possible.

I appreciate the reply! If you need any more info let me know.

I've tagged ChalkOne on the FoundryVTT #module-troubleshooting discord channel with another screenshot showing a UI formatting bug as well in v9.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

— Reply to this email directly, view it on GitHub https://github.com/jopeek/fvtt-loot-sheet-npc-5e/issues/207#issuecomment-999774731, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVRQRS4PEFARVSASJMJ7NVDUSII4NANCNFSM5KRYBQKQ . You are receiving this because you authored the thread.Message ID: @.***>

ctbritt commented 2 years ago

having same bug with only LootSheet enabled.

System stats: OS: macOS Browser: chrome Foundry 9.238 LootSheet: 3.3.81

Also thanks for your notes on my previous question!

DanielBoettner commented 2 years ago

I just tested again with

What I would like to see to test this.

Sytem settings for Lootsheet

Screenshots of the lootsheet settings (not the specific sheet) to reproduce that part.

GM Sidebar settings

A screenshot of the GM sidebar in the newest version. Want to see if the permission icons are visible on the players If you know how, maybe also a screenshot of the html structure.

Example (click/open to see bigger version): image

The html attributes should all be filled as they are used in the code to identify/find elements. And hold the data.

germandrewboat commented 2 years ago

Mis-click on closing. I'm not on .8.9 so I can't test this, but glad to hear progress is being made!

DanielBoettner commented 2 years ago

@germandrewboat could you test if v3.4.1 solves this issue?

It has to fixes:

I still couldn't test this on v9. But I think getEmbeddedDocument should be uptodate call we need.

Please feel encouraged to report v9 bugs with the latest versions of the lootsheet. As I'd like to solve most of them in a v0.8.9 environment before focusing on bugs we can only fix with v9.

germandrewboat commented 2 years ago

@germandrewboat could you test if v3.4.1 solves this issue?

It has to fixes:

  • css wasn't loaded on linux in 3.3.8.x to 3.3.9.x
  • updates a method from getEmbeddedEntity to getEmbeddedDocument

I still couldn't test this on v9. But I think getEmbeddedDocument should be uptodate call we need.

Please feel encouraged to report v9 bugs with the latest versions of the lootsheet. As I'd like to solve most of them in a v0.8.9 environment before focusing on bugs we can only fix with v9.

Sure! Gimme a bit and I'll come back with a report.

EDIT: The formatting looks wonderful. From what I can tell that is broken still:

1) Bulk Update does not apply as I'm assuming it is supposed to be single click, then changes for everyone; so you have to apply it manually which is a pain because that window closes every time you assign a individual permission. 2) Player does not, even when having observer permission, have access to the list of inventory items. It is just an empty window when trying to double click the token on the map.

First screenshot is player view, 2nd is GM.

player_view lootsheetupdates

rtschaefer commented 2 years ago

Loot Sheet was working fine, but now players cannot purchase items from merchants. They can click to buy, they can confirm the number of items, but no transfer happens...either transfer of coin or of the item being bought. I really hope this gets fixed soon.

Also... I am not happy with the hidden settings panel in the DM version of the sheet. That's not a feature for me...it forces me to constantly have to re-open in again and again and again as I am working on the sheet.

DanielBoettner commented 2 years ago

@germandrewboat thanks for the report. Does the chrome console on either end gm/player put out any errors?

@rtschaefer This is also on v9?

Regarding the Sidebar. I see you pain regarding it closing over and over. I will try to change in the future in a way that it doesn't occur so often. I already have a solution in mind.

This wont be as fix as the bugfixes in the last days. Today I want to foucs on releasing the loot populator module in a new version. (no clue on v9 state currently)

I talked with @jopeek and somewhere along the v9 way we likely merge the populator into the sheet. This should make assigning rolltables and formulas manually feel very obsolete.

germandrewboat commented 2 years ago

@germandrewboat thanks for the report. Does the chrome console on either end gm/player put out any errors?

  • Bulk update should update for all the players listed underneath.
  • Does the access only happen for merchants or for all sheets?

@rtschaefer This is also on v9?

Regarding the Sidebar. I see you pain regarding it closing over and over. I will try to change in the future in a way that it doesn't occur so often. I already have a solution in mind.

This wont be as fix as the bugfixes in the last days. Today I want to foucs on releasing the loot populator module in a new version. (no clue on v9 state currently)

I talked with @jopeek and somewhere along the v9 way we likely merge the populator into the sheet. This should make assigning rolltables and formulas manually feel very obsolete.

edit: So far it's on merchant sheets that the player didn't see the inventory...however after changing nothing now the player can...

For loot sheets: When trying to bulk change import permissions, I get the attached error in console. notiterable

I also get a generic "No active character for user" message even when a player user clearly has permissions of a character on the same scene as the Lootable actor.

So, no error from the Player side, the buy request seems to send when checking F12 console, but the GM gets this error.

buy_request

DanielBoettner commented 2 years ago

For the user (permissions) I will have to investigate what changed on handling of the users.

The updateEmbeddedEntity() is now (locally) already changed to updateEmbeddedDocument(). I hoped I catched all of these before already but evidently not.

DanielBoettner commented 2 years ago

I also found the reason for the permissions problem. Currently going through the code and check it against https://gitlab.com/foundrynet/foundryvtt/-/issues/5783.

in other words. Vor v9 to work I think mostly we will have to find all the deprecated/deleted methods and replace them by their newname.

germandrewboat commented 2 years ago

I also found the reason for the permissions problem. Currently going through the code and check it against https://gitlab.com/foundrynet/foundryvtt/-/issues/5783.

in other words. Vor v9 to work I think mostly we will have to find all the deprecated/deleted methods and replace them by their newname.

Yes, entity's were deprecated in favor of document. I believe Atropos and others mentioned it on all the v9 Development streams in prep for v9 release. Glad to hear progress is being made! Mind pinging me when you have an update ready so I can test again?

DanielBoettner commented 2 years ago

Feel free to contact me via Discord JackPrince# 0494 I can send you my builds if this helps. I wont push/release the updates for that as fast as we do for the 0.8 bugfixes.

DanielBoettner commented 2 years ago

This seems to work in most parts even in v9

~3421.zip~

THIS not released yet! You have to install it manually.

@rtschaefer @ctbritt @jopeek

DanielBoettner commented 2 years ago

lootsheetnpc5e v3.4.2.5

For a manual install. This is my latest dev build. It seems to work in v8 and v9.

thanks to @germandrewboat for the help with debugging in v9, much appreciated.

germandrewboat commented 2 years ago

lootsheetnpc5e v3.4.2.5

For a manual install. This is my latest dev build. It seems to work in v8 and v9.

thanks to @germandrewboat for the help with debugging in v9, much appreciated.

Thank you for the help as well! I'll try and start working with RollTables so I can find any other bugs.

rtschaefer commented 2 years ago

@germandrewboat thanks for the report. Does the chrome console on either end gm/player put out any errors?

No in game error messages at all (see F12 console below). The player receives the normal dialogue box of how many items in the stack they wish to buy, but then nothing happens! No money is exchanged, and the item is not transferred to the player's inventory. This always worked perfectly in the past.

  • Bulk update should update for all the players listed underneath.
  • Does the access only happen for merchants or for all sheets?

All Merchant sheets, even brand new ones that I made, report out this message. Loot sheets on fallen monsters works perfectly fine. It is just the Merchants that are broken. Here is the F12 code I initially received.

Item Macro | DEBUG | changeButtonExecution : Object foundry.js:3272 You are referencing DocumentSheet#entity which has been deprecated in favor of DocumentSheet#document get entity @ foundry.js:3272 LootsheetNPC5e.js:460 lootsheetnpc5e Sending buy request to DM Object DevTools failed to load source map: Could not load content for https://assets.forge-vtt.com/bazaar/modules/forien-unidentified-items/0.3.17/styles/style.css.map: HTTP error: status code 404, net::ERR_HTTP_RESPONSE_CODE_FAILURE

It is odd that it referenced Forien's Unidentified Items, so I uninstalled the module, reset the merchant with new items, and tried all over again. Unfortunately, it still does not work. Same results. The following was noted from the console:

console.warn("You are referencing DocumentSheet#entity which has been deprecated in favor of DocumentSheet#document");

@rtschaefer This is also on v9?

I am on v8.9 of Foundry until v9 is stable.

Regarding the Sidebar. I see you pain regarding it closing over and over. I will try to change in the future in a way that it doesn't occur so often. I already have a solution in mind.

This wont be as fix as the bugfixes in the last days. Today I want to foucs on releasing the loot populator module in a new version. (no clue on v9 state currently)

I talked with @jopeek and somewhere along the v9 way we likely merge the populator into the sheet. This should make assigning rolltables and formulas manually feel very obsolete.

Great news! I am launching a new campaign Jan.7th so hopefully the basic functions will work, but until then, I like what you plan to do with the module.

rtschaefer commented 2 years ago

I also forgot to mention that players can still loot chests and fallen creatures perfectly with no errors. It is just the merchant sheet that does not work. Also, I am noticing that some merchants have inexplicably reverted back to NPC stat sheets, forcing me to reset their character sheets back to LootSheet. Does not seem to be a pattern to it, this is just something that happens. Finally, some merchant sheets were reverting to LOOT instead of merchant, which is scary. LOL

DanielBoettner commented 2 years ago

I think their is some kind of race condition with switching the sheets.

Regarding merchants / buying items. I tested this earlier with another user in v0.8.9 and v9 and did not encounter the problems.

However I currently update the handling of the item transfer to prevent duplicates. I make sure to test buying once more.

console.warn("You are referencing DocumentSheet#entity which has been deprecated in favor of DocumentSheet#document");

I don't think this is related to lootsheet as I can't find any reference like that anymore. But I will investigate.

rtschaefer commented 2 years ago

Tonight, the merchants are appearing to my players as a Tidy NPC sheet, even though for me it appears as a Loot Sheet. I tried to delete the merchant token and then set up a new one, have the player leave the scene and then reenter (etc.) and still, it shows up for the player as an NPC character, not a merchant.

It was working fine until the most recent update.

DanielBoettner commented 2 years ago

Sorry to hear that. I will try to check for these kind of errors in v0.8.9 over the next days.

I have some optimizations already for buying and looting. But will check again for the sheet behavior.

Do you know if you players did see any errors? Did you have any errors in the console when the sheet was switched?