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

[SM64] Fix persistent block spam #434

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

jesusyoshi54
Copy link
Collaborator

macros were parsing comments even though they were on a new line, so I made the macro not see a comment with a newline character unless there was a backslash

[^()]*
.*? # match as few chars as possible before macro name
(?P<macro_name>\w+) #group macro name matches 1+ word chars
\s* # allows any number of spaces after macro name
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you make this lazy (was causing extra steps according to the debugger)

Suggested change
\s* # allows any number of spaces after macro name
\s*? # allows any number of spaces after macro name

Copy link
Collaborator

@Lilaa3 Lilaa3 left a comment

Choose a reason for hiding this comment

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

I deleted my last comments and rethought my own suggestions, tell me what you think

(\s*,\s*|\s*)
(?P<comment>
//.*$
(\s*,\s*|\s*) # group of whitespace and comma and whitespace (??)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(\s*,\s*|\s*) # group of whitespace and comma and whitespace (??)
(\s*?,)?[^\n]*? # capture a comma, including white space trailing except for new lines following the comma

//.*$
(\s*,\s*|\s*) # group of whitespace and comma and whitespace (??)
(?P<comment> # comment group
([^\n]|\s*?\\\s*?\n)//.*$ # two // and any number of chars and str or line end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
([^\n]|\s*?\\\s*?\n)//.*$ # two // and any number of chars and str or line end
([^\n]*?|\s*?\\\s*?\n)//.*$ # two // and any number of chars and str or line end

|
/\*.*\*/
)?
([^\n]|\s*?\\\s*?\n)/\*.*\*/ # a /*, any number of chars and a */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
([^\n]|\s*?\\\s*?\n)/\*.*\*/ # a /*, any number of chars and a */
([^\n]*?|\s*?\\\s*?\n)/\*[\s\S]*?\*/ # a /*, any number of chars (including new line) and a */

@Lilaa3 Lilaa3 added bug Something isn't working sm64 Has to do with the Super Mario 64 side labels Aug 25, 2024
Copy link
Collaborator

@Lilaa3 Lilaa3 left a comment

Choose a reason for hiding this comment

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

Works!

@Lilaa3 Lilaa3 merged commit 2e5dd87 into Fast-64:main Sep 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sm64 Has to do with the Super Mario 64 side
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants