search

mclemente / fvtt-module-polyglot

Talk to others using a language you can speak and scrambles text you can't understand.
MIT License
41 stars 48 forks source link

Registration of providers with API is broken #294

Closed AterIgnis closed 1 year ago

AterIgnis commented 1 year ago

Describe the bug Functions registerModule(moduleId, languageProvider) and registerSystem(languageProvider) call #register(source, type, languageProvider) but provide two arguments instead of three (type is missing)

To Reproduce

  1. Add and enable module using API to register language provider
  2. Error in console, provider not registered

Expected behavior Language provider registered successfully

Screenshots image

Additional context Add any other context about the problem here.

mclemente commented 1 year ago

I've pushed a new release that should fix this issue. Let me know if you run into any other issues.

mclemente commented 1 year ago

Also, what system/module is that? Next major release should have breaking changes but I don't have any external modules to test the impact on the Language Provider changes.

AterIgnis commented 1 year ago

Hello, i've tested new version and it still have some issues. I suppose in https://github.com/mclemente/fvtt-module-polyglot/blob/8086cf56ec13eaa483d05906610ae823608596f6/module/api.js#L101 there is need to change to this.providers[providerInstance.id] = providerInstance; as availableLanguageProviders seems to not be used anymore and also .id is a part of instance, not a base class.

As for system/module, we use D35E system with custom module for homebrews, we had some times in the past where some of the language settings were reset (possibly due to module load errors leading to custom languages missing from system sometimes, but i am not sure if this is a cause), so I decided to make LanguageProvider for it few days ago, using example from https://github.com/mclemente/fvtt-module-polyglot/wiki/API (which needs updating too, ex. last line should be calling game.polyglot.api.registerModule now but full example and detailed walkthrough have different calls in that place)

mclemente commented 1 year ago

Ok, latest release should have that line fixed too.

Yes, the documentation is a bit outdated now with the changes. There are more upcoming changes to the Language Provider, so I held out on changing it for now. E.g. the originalAlphabets and originalTongues are being merged into a languages getter, which will also allow to set the way the randomization is done for each language instead of global setting.

AterIgnis commented 1 year ago

Looks like it is working now, thanks.