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.

Sign up for GitHub

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

Jump to bottom

Const eval for some math procs #1202

Merged
merged 22 commits into from
Oct 4, 2023
Merged

Const eval for some math procs #1202

merged 22 commits into from
Oct 4, 2023

Conversation

pali6
Copy link
Contributor

@pali6 pali6 commented May 1, 2023

Added only those that BYOND considers const-evaluable (no round for example).

@pali6
Copy link
Contributor Author

pali6 commented May 1, 2023

Will try to actually make them const-evaluable. Will reopen then.

@pali6 pali6 closed this May 1, 2023
@pali6 pali6 changed the title Add some math procs to IsValidRighthandSide Const eval for some math procs May 1, 2023
@pali6 pali6 reopened this May 1, 2023
@pali6
Copy link
Contributor Author

pali6 commented May 1, 2023

Huh, locally tests were passing. Will check what's wrong tomorrow. Also I should remove the native proc implementations now.

@pali6
Copy link
Contributor Author

pali6 commented May 2, 2023

The unit test uses approximate equality because of the fact that .NET's trigonometry functions can and do behave differently on different platforms. See the remark here. There is apparently a 1.1920929E-07 difference between Windows and Linux (or at least between my computer and the github actions server). Maybe at some point we will have to use something else than MathF and Math to avoid this issue.

@github-actions
Copy link

github-actions bot commented May 5, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

wixoaGit
wixoaGit requested changes Oct 2, 2023
View reviewed changes
DMCompiler/Compiler/DM/DMAST.cs Outdated Show resolved Hide resolved
DMCompiler/Compiler/DM/DMAST.cs Outdated Show resolved Hide resolved
DMCompiler/DM/Expressions/Constant.cs Outdated Show resolved Hide resolved
DMCompiler/DM/Expressions/Constant.cs Outdated Show resolved Hide resolved
DMCompiler/DM/Expressions/Constant.cs Outdated Show resolved Hide resolved
OpenDreamRuntime/Procs/DMOpcodeHandlers.cs Outdated Show resolved Hide resolved
OpenDreamRuntime/Procs/DMOpcodeHandlers.cs Outdated Show resolved Hide resolved
OpenDreamRuntime/Procs/DMOpcodeHandlers.cs Outdated Show resolved Hide resolved
OpenDreamRuntime/Procs/Native/DreamProcNativeRoot.cs Outdated Show resolved Hide resolved
Content.Tests/DMProject/Tests/Expression/const_eval.dm Outdated Show resolved Hide resolved
@pali6 pali6 requested a review from wixoaGit October 3, 2023 16:05
@wixoaGit wixoaGit merged commit 4017375 into OpenDreamProject:master Oct 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wixoaGit wixoaGit wixoaGit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pali6 @wixoaGit @amylizzle