Skip to content

Add back removal of last period in multiattack parsing#41

Closed
Merudo wants to merge 1 commit into
Stendarpaval:mainfrom
Merudo:main
Closed

Add back removal of last period in multiattack parsing#41
Merudo wants to merge 1 commit into
Stendarpaval:mainfrom
Merudo:main

Conversation

@Merudo

@Merudo Merudo commented Sep 23, 2021

Copy link
Copy Markdown
Contributor
  • The last period of Multiattack description is now removed again before being parsed

- The last period of Multiattack description is now removed again before being parsed
@Stendarpaval

Copy link
Copy Markdown
Owner

Ah, good initiative of you to make a PR for that last period! Thank you.

However, since I was also working on something else (as you can read about in v0.3.9's release notes), I already re-added the lines that remove the period. That's what I was referring to in the review of your previous PR, but I wasn't very clear about it. 😅

Let me know if there are any bugs with v0.3.9 for your multiattack descriptions.

In any case, I'll go ahead and close this PR. :)

@Merudo

Merudo commented Sep 23, 2021

Copy link
Copy Markdown
Contributor Author

I think it makes more sense to remove the period after other HTML tags have been removed. That way the period gets deleted even if there is other formatting (<div>, more <p>, etc.).

I'll keep this in mind while using Mob Tool, if an attack is not recognized I'll check if removing the period solves it :)

@Stendarpaval

Copy link
Copy Markdown
Owner

That's a valid point. Even if you don't encounter situations where attacks aren't recognized, I might still follow your suggestion when I refactor multiattack.js (which I plan to do eventually in order to address other shortcomings of the algorithm).

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.

2 participants