Skip to content

Fix a few issues with parsing drawings#9

Merged
CoffeeFlux merged 2 commits intoTypesettingTools:masterfrom
arch1t3cht:drawing_fix
Jan 1, 2023
Merged

Fix a few issues with parsing drawings#9
CoffeeFlux merged 2 commits intoTypesettingTools:masterfrom
arch1t3cht:drawing_fix

Conversation

@arch1t3cht
Copy link
Member

@arch1t3cht arch1t3cht commented Dec 28, 2022

This can mostly be reviewed commit by commit, with explanations in the commit messages.
The handling of the c command isn't perfect yet, especially since it should only be relevant when following s commands, and those aren't even recognized yet. For that reason, I just made this minimally invasive fix and nothing more. If parsing of s is ever added, c can be adjusted.

The constructor should be given an empty argument list.
The if block cannot be simply removed entirely, since then any
coordinates following the 'c' would cause an infinite loop since the
Close command has parameter count 0.
These might have worked for some moonscript version in the past (I don't
know for sure), but they don't appear in the reference, and they do not
work on Aegisub builds currently in use or current moonscript
distributions on Linux. "a, b += 1" just compiles to "local a = a + 1".

DrawBezier.getTagParams = (precision = @__tag.precision) =>
x1, y1, x2, y2, x3, y3 = @p1.x, @p1.y, @p2.x, @p2.y, @p3.x, @p3.y
x1, x2, x3, y1, y2, y3 %= @__tag.mod if @__tag.mod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch. Did you skim through to see if this happens elsewhere, or should I do that? It's also probably worth quickly auditing DepCtrl for this pattern...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I grepped all of assf for something like ,.*[^ =!]= to find these, so hopefully these are all occurrences here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like DepCtrl doesn't have any of these.

@CoffeeFlux CoffeeFlux merged commit 8fbb4f4 into TypesettingTools:master Jan 1, 2023
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