Skip to content
This repository was archived by the owner on Mar 12, 2025. It is now read-only.

368 Add address mode resolving tests#369

Merged
flamion merged 2 commits intomainfrom
368-fix-address-resolving
Mar 11, 2025
Merged

368 Add address mode resolving tests#369
flamion merged 2 commits intomainfrom
368-fix-address-resolving

Conversation

@flamion
Copy link
Copy Markdown
Contributor

@flamion flamion commented Mar 7, 2025

No description provided.

@flamion flamion requested a review from nikkischnelle March 7, 2025 15:34
@flamion flamion self-assigned this Mar 7, 2025
@flamion flamion added the preview labeled PRs will be deployed label Mar 7, 2025
@flamion flamion temporarily deployed to 368-fix-address-resolving March 7, 2025 15:38 — with GitHub Actions Inactive
@flamion flamion added preview labeled PRs will be deployed and removed preview labeled PRs will be deployed labels Mar 7, 2025
0815sailsman
0815sailsman previously approved these changes Mar 7, 2025
@flamion flamion marked this pull request as draft March 7, 2025 16:37
@flamion flamion force-pushed the 368-fix-address-resolving branch from a633beb to 112ad16 Compare March 9, 2025 08:23
@flamion flamion temporarily deployed to 368-fix-address-resolving March 9, 2025 08:29 — with GitHub Actions Inactive
@flamion flamion force-pushed the 368-fix-address-resolving branch 4 times, most recently from 6bc4b70 to f2aada3 Compare March 9, 2025 09:10
@flamion flamion temporarily deployed to 368-fix-address-resolving March 9, 2025 09:15 — with GitHub Actions Inactive
@flamion flamion force-pushed the 368-fix-address-resolving branch from 4e7b7d9 to 20d5ecf Compare March 9, 2025 09:24
@flamion flamion temporarily deployed to 368-fix-address-resolving March 9, 2025 09:30 — with GitHub Actions Inactive
@nikkischnelle nikkischnelle added preview labeled PRs will be deployed and removed preview labeled PRs will be deployed labels Mar 10, 2025
@flamion flamion requested a review from 0815sailsman March 10, 2025 14:15
@flamion flamion marked this pull request as ready for review March 10, 2025 14:15
@flamion flamion temporarily deployed to 368-fix-address-resolving March 10, 2025 14:22 — with GitHub Actions Inactive
Comment thread backend/shork/src/main/kotlin/internal/memory/ICore.kt Outdated
@flamion flamion force-pushed the 368-fix-address-resolving branch from 5dc6fab to 5b986ca Compare March 10, 2025 15:00
@flamion flamion temporarily deployed to 368-fix-address-resolving March 10, 2025 15:06 — with GitHub Actions Inactive
@flamion flamion requested a review from nikkischnelle March 10, 2025 15:07
@0815sailsman
Copy link
Copy Markdown
Contributor

Do you have an illustrative pair of programs that did not work before, that works now?

@flamion
Copy link
Copy Markdown
Contributor Author

flamion commented Mar 10, 2025

Do you have an illustrative pair of programs that did not work before, that works now?

Yes, they are anchored in the tests and have been pulled from https://corewar-docs.readthedocs.io/en/latest/redcode/addressing_modes/#a-post-increment-indirect and https://vyznev.net/corewar/guide.html#start_modes and been slightly modified. Theoretically the scanner should be broken without this

Copy link
Copy Markdown
Collaborator

@PietHelzel PietHelzel left a comment

Choose a reason for hiding this comment

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

LGTM

@flamion flamion force-pushed the 368-fix-address-resolving branch from 5b986ca to 6c90be7 Compare March 10, 2025 15:28
@flamion flamion temporarily deployed to 368-fix-address-resolving March 10, 2025 15:34 — with GitHub Actions Inactive
@flamion flamion temporarily deployed to 368-fix-address-resolving March 10, 2025 15:48 — with GitHub Actions Inactive
@flamion
Copy link
Copy Markdown
Contributor Author

flamion commented Mar 10, 2025

Theoretically those should look different between mainline and this branch:

DAT #0, #4
MOV 0, >-1
jmp 0

@flamion flamion force-pushed the 368-fix-address-resolving branch from cb30013 to 6c90be7 Compare March 10, 2025 15:49
@flamion flamion temporarily deployed to 368-fix-address-resolving March 10, 2025 15:54 — with GitHub Actions Inactive
0815sailsman
0815sailsman previously approved these changes Mar 10, 2025
Copy link
Copy Markdown
Contributor

@0815sailsman 0815sailsman left a comment

Choose a reason for hiding this comment

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

sure

@flamion flamion force-pushed the 368-fix-address-resolving branch from f7c0ed1 to a843f03 Compare March 10, 2025 17:18
@flamion flamion changed the title 368 fix address resolving 368 Add address mode resolving tests Mar 10, 2025
@flamion
Copy link
Copy Markdown
Contributor Author

flamion commented Mar 10, 2025

After further discussion and further checking the source code attached in the '94 standard, it seems that https://vyznev.net/corewar/guide.html#start_modes contains incorrect information and the original implementation was correct after all.
Changing this to simply add the until now missing tests

Copy link
Copy Markdown
Contributor

@0815sailsman 0815sailsman left a comment

Choose a reason for hiding this comment

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

sure

@flamion flamion temporarily deployed to 368-fix-address-resolving March 10, 2025 17:24 — with GitHub Actions Inactive
Copy link
Copy Markdown
Collaborator

@PietHelzel PietHelzel left a comment

Choose a reason for hiding this comment

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

lgtm

@flamion flamion merged commit a843f03 into main Mar 11, 2025
11 checks passed
@flamion flamion deleted the 368-fix-address-resolving branch March 11, 2025 16:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Backend/Interpreter preview labeled PRs will be deployed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants