Skip to content

feat: add recipient for specific language #250

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

Merged
merged 2 commits into from
Feb 8, 2025
Merged

Conversation

pro470
Copy link
Contributor

@pro470 pro470 commented Feb 8, 2025

This is useful when you have BMS as a plugin system and want multiple languages implemented. You also want the events for different languages to be sent on different schedules. You need all language scripts to be on a dedicated entity and can not put them all on one “Language” entity.

@pro470
Copy link
Contributor Author

pro470 commented Feb 8, 2025

This allows also us to have the luascriptplugin and all the system it needs in one plugin. And can create a more creative schedules with it

@makspll
Copy link
Owner

makspll commented Feb 8, 2025

Hi and thanks for contributing 👋!

This is a good point, I wonder if event handlers should by default only handle the language their plugin is parameterized by anyway? I.e.

if P::Language != script.language {
    continue
}

Would that solve your issue?

@Joakker
Copy link
Contributor

Joakker commented Feb 8, 2025

This is a good point, I wonder if event handlers should by default only handle the language their plugin is parameterized by anyway?

That would be the logical option. However it would also necessitate keeping track of which ScriptId has which language

@pro470
Copy link
Contributor Author

pro470 commented Feb 8, 2025

I think this would not solve my issue because when I want all systems also the event send systems to be in one plugin I can't send dedicated events to a specific language.
This was also the problem I had. When I have event send systems in each language the first event handler system would run the Lua script twice because new_for_all sends it to all languages. With the recipient, I can have the send systems in their plugin but only for the language.
So the issue I have is not on the event handler but rather how an event is being sent.

@makspll
Copy link
Owner

makspll commented Feb 8, 2025

That makes sense, happy to add merge this then and if people have issues with this approach we can re-visit.

Could you run cargo fmt to pass CI?

@makspll makspll added the needs documentation For features or changes which still need changes in the book label Feb 8, 2025
@pro470
Copy link
Contributor Author

pro470 commented Feb 8, 2025

That makes sense, happy to add merge this then and if people have issues with this approach we can re-visit.

Could you run cargo fmt to pass CI?

I'm happy to help when people have Issues. 😁
I'm also completely open to have a different implementation for this solution as well.

@makspll makspll merged commit b322ba4 into makspll:main Feb 8, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs documentation For features or changes which still need changes in the book
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants