Skip to content
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

Update region logic #2240

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Update region logic #2240

merged 1 commit into from
Jun 27, 2023

Conversation

scooterboo
Copy link
Contributor

Description

Sometimes, there's triggers with actions, but no conditions. It's more accurate to do matching based off of name rather than condition.

Issues fixed by this PR

Unfortunately, I've only seen this fix the slimes at group 133104058 at 629.781 200.000 573.870
That group is a pretty good example of this happening, though.

Type of changes

  • Bug fix
  • New feature
  • Enhancement
  • Documentation

Checklist:

  • My code follows the style guidelines of this project
  • My pull request is unique and no other pull requests have been opened for these changes
  • I have read the Contributing note and Code of conduct
  • I am responsible for any copyright issues with my code if it occurs in the future.

@KingRainbow44 KingRainbow44 merged commit 83d447c into Grasscutters:development Jun 27, 2023
@scooterboo scooterboo deleted the patch-3 branch June 27, 2023 06:57
@eternalcomet
Copy link
Contributor

I think this is not a proper fix. We shouldn't treat EVENT_ENTER_REGION differently. The condition_EVENT_ENTER_REGION_xxx is checked by lua scripts rather than java side. So we need to add missing ScriptLib functions like GetRegionConfigId to make these triggers triggered more naturally. However, sometimes the condition is empty in some triggers, not only for EVENT_ENTER_REGION. So we need to find a general way to handle the event trigger when the condition is empty for other eventTypes, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants