Skip to content

CSE Machine: Added animation for spread instruction #3114

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
Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

CSE Machine: Added animation for spread instruction #3114

wants to merge 15 commits into from

Conversation

alsonleej
Copy link
Contributor

@alsonleej alsonleej commented Mar 12, 2025 •
edited
Loading

Description

Partially deals with #3110, further work can be done on array access out of bounds

Type of change

  • New feature (non-breaking change which adds functionality)
20250312-1921-34.2597022.mp4

Checklist

  • I have tested this code
  • I have updated the documentation

@coveralls
Copy link

coveralls commented Mar 13, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 14290737373

Details

  • 0 of 49 (0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 31.004%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/features/cseMachine/animationComponents/ArrayAccessAnimation.tsx 0 6 0.0%
src/features/cseMachine/CseMachineAnimation.tsx 0 11 0.0%
src/features/cseMachine/animationComponents/ArraySpreadAnimation.tsx 0 32 0.0%
Files with Coverage Reduction New Missed Lines %
src/features/cseMachine/animationComponents/ArrayAccessAnimation.tsx 1 0.0%
Totals Coverage Status
Change from base Build 14256181806: -0.1%
Covered Lines: 4892
Relevant Lines: 14883
💛 - Coveralls

@NhatMinh0208
Copy link
Contributor

NhatMinh0208 commented Mar 14, 2025 •
edited
Loading

Looking at it, might take a while though since I'm busy

s-kybound
s-kybound requested changes Mar 17, 2025
View reviewed changes
Copy link
Member

@s-kybound s-kybound left a comment

Choose a reason for hiding this comment

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

Looks good, but please remove the print statements.

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 17, 2025
View reviewed changes
@alsonleej alsonleej mentioned this pull request Mar 20, 2025
Merged
3 tasks
@alsonleej alsonleej requested a review from s-kybound March 25, 2025 04:13
@martin-henz martin-henz changed the title Added animation for spread instruction CSE Machine: Added animation for spread instruction Apr 1, 2025
RichDom2185
RichDom2185 reviewed Apr 6, 2025
View reviewed changes
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

Would be good if you have a screen recording showing the new animation in action!

@martin-henz martin-henz requested a review from RichDom2185 April 15, 2025 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AustinHuang1203 AustinHuang1203 Awaiting requested review from AustinHuang1203

@NhatMinh0208 NhatMinh0208 Awaiting requested review from NhatMinh0208

@s-kybound s-kybound Awaiting requested review from s-kybound

@RichDom2185 RichDom2185 Awaiting requested review from RichDom2185

Requested changes must be addressed to merge this pull request.

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

Successfully merging this pull request may close these issues.
6 participants
@alsonleej @coveralls @NhatMinh0208 @RichDom2185 @s-kybound @martin-henz