vigoren / foundryvtt-simple-calendar

A simple calendar module for the FoundryVTT system
https://simplecalendar.info/
MIT License
48 stars 36 forks source link

[BUG] Maximum Call Stack Size exceeded #613

Open TimorSol opened 5 months ago

TimorSol commented 5 months ago

Describe the bug Simple Calendar seems to create a overflow bug whenever I roll any item from the character sheet.

To Reproduce Steps to reproduce the behavior:

  1. Open a5e game system, open any character sheet.
  2. Roll any item that requires a dice roll.
  3. See a staggering error in the console.

Expected behavior While the overflow does not kill the process, it's pretty extensive.

Screenshots simcal error

Foundry Information

Browser (please complete the following information):

Additional context Bug found with Find the Culprit, confirmed manually later. Seen at least one person posting the same issue with Simple Calendar using PF2e in Foundry discord.

Forien commented 5 months ago

I have the same bug on wfrp4e with my module forien-armoury.

Context

Tests in wfrp4e save data of the whole Test class in Chat Message's flags in order to be able to recreate Test for purposes of rerolling etc. This includes data objects of items, rolled values etc.

I did not encounter the error using wfrp4e normally.

Error

Error screenshot from Console ![image](https://github.com/vigoren/foundryvtt-simple-calendar/assets/11304207/6f1ea598-aea6-4ebc-bd23-ebb1d9fab941)

Error in my case is very highly specific though:

  1. Must be on v12 (I'm using 12.327) – does not appear on v11.315
  2. Must be using my Forien's Armoury module
  3. Must be trying to use Magic Scroll item type from my module
    • said scroll must be attached to a spell from very specific lores, like "metal" or "heavens". Lores like "life" or "death" don't cause this error
    • what is weird is that lores that cause issues have one thing in commong: they don't generate chat buttons. But I have no idea how it could even impact this updateSource, because in flags the only difference is a sting value on spell item data model...
      ( if needed can help with more detailed reproduction steps )

What is weird is that only single string value can be different and that makes whole difference between there being an error and there not being an error. I was expecting something else, like circular references or something.

Investigation & potential solution?

Anyway, I narrowed the culprit to this function: https://github.com/vigoren/foundryvtt-simple-calendar/blob/f09bb9697df4e1c58d8758726d8a35089bc77644/src/classes/chat/chat-timestamp.ts#L8-L33

And I feel like putting all flags in changes is not very optimal. Because you force Foundry to compare sometimes big data structures even though you are not really setting them.

Changing last line like this removed the issue altogether, which may point to more proper solution:

-chatMessage.updateSource({ flags: messageFlags });
+chatMessage.updateSource({ "flags.foundryvtt-simple-calendar.sc-timestamps": flagData });

Can also confirm that the error does not prevent neither system or my module from doing our things. All it prevents is SC from adding a timestamp.

vigoren commented 5 months ago

Thanks! That does help, I just need to do some testing to see if that fixes the issue properly or if more is needed.

TimorSol commented 1 month ago

Help, it's again, as the classic used to say. This error was gone for a decent time, but suddenly reappeared. I'm not aware of any significant changes done in either the system nor module (I think it wasn't even updated?), so maybe it's a core issue?

V 12.331, A5e 19.26

obraz

full bug log.txt

Cosmic-Lattee commented 2 weeks ago

So I recently started seeing this bug while running the BRP system (https://github.com/Genii-Locorum/brp). Any time I roll anything over there this same (or at least very similar) error will appear in the console. It breaks some stuff in the system (prevents combat cards from working right) so for now I can't use it.

Just wanted to say that this bug is happening with other systems as well.

Cosmic-Lattee commented 1 week ago
-chatMessage.updateSource({ flags: messageFlags });
+chatMessage.updateSource({ "flags.foundryvtt-simple-calendar.sc-timestamps": flagData });

So with a bit of digging I eventually found this in the module, and changing it to the proposed line of code seems to fix the issue for me as well even though I am using a different system.