Issue with character import

[zeldafreak6245]

Source: Discord OP: Tastyhaggis Date: 06/25/2023

Describe the bug Hi all! Whenever I import characters from the 5E website something odd happens on the VTT. The first character tends to go through fine and it pulls through their classes and stats wonderfully.

Subsequent characters however have an issue where they are reimported and the prompt for advancement includes the PREVIOUS character's classes as well as their own.

To Reproduce

1. Import first character without any issue. In this example a FIGHTER. Advancement allows the selection/processing of level upgrades.
2. Import second character; a BERSERKER. When importing the advancement prompt appears.
3. Berserker has extra steps which turns out the FIGHTER and BERSERKER advancements have complied meaning rather than a level one BERSERKER they are both a FIGHTER and BERSERKER.
4. Subsequent imports compound the issue by adding levels from the previous classes.

Expected behavior Importing to work properly.

**Foundry Information Foundry version: V10, V11 SW5e version: 2.4.7

[afwolfe]

I am also encountering this in the 2.2.2 beta on Foundry V11. I did some testing and this appears to be related to the static variables in the CharacterImporter class:

https://github.com/sw5e-foundry/sw5e/blob/master/module/character-importer.mjs#L7

  static _itemsWithAdvancement = [];
  static _actor = null;

When importing multiple characters, the _itemsWithAdvancement is not cleared and applies the advancements of any previously imported characters. The _actor appears to be updated with the new Actor before any other methods using it are called, but could have some strange behaviors if the user starts another import before the current one is finished.

I would be interested in fixing this but would like to discuss the approach. Here are two thoughts I had, but there may be a better solution:

• The simple fix would be to reset these two objects at the beginning (or end) of an import/transform() call, but this doesn't solve any errors or side-effects if the user attempts to start another import before the previous one is finished.
• Should these two static variables become local to the transform method and instead be passed down into the helper functions that need them? I think this could solve this bug and potentially prevent any issues caused by accidentally starting a second import before the previous has finished.

[Ikaguia]

They were local variables before, and I changed them to static to simplify the code. (which in hindsight was not that good of an idea) I believe the best way would actually be to change the variables and all functions to be instance variables and functions, also changing the code that calls the importer to create a new instance when doing so. This would solve the issue of advancements stacking, and also any other issues related to starting a second import while the first is underway

[afwolfe]

I agree that approach seems more appropriate and I can give it a try. I had started thinking through what passing in the objects as arguments and mutating them would look like and it was becoming quite ugly.