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

fix memory opcodes #3

Merged
merged 2 commits into from
Mar 25, 2024
Merged

fix memory opcodes #3

merged 2 commits into from
Mar 25, 2024

Conversation

impaulsible
Copy link
Contributor

Die opcodes von allen memory-Befehlen wurden nicht angezeigt, weil die nicht bei ITYPE_OPCODES dabei waren.

Copy link

netlify bot commented Mar 18, 2024

Deploy Preview for mips-assembler ready!

Name Link
🔨 Latest commit fe7bd80
🔍 Latest deploy log https://app.netlify.com/sites/mips-assembler/deploys/6601c9e3c0f2f7000858e82f
😎 Deploy Preview https://deploy-preview-3--mips-assembler.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mclrc
Copy link
Owner

mclrc commented Mar 20, 2024

Krass ist mir gar nicht aufgefallen, guter catch! Würde tatsächlich die memory opcodes auch in die itype opcodes reinspreaden, so wie mit den branches, sind ja beides auch itypes :)

@impaulsible
Copy link
Contributor Author

Ich dachte die memory opcodes sind absichtlich nicht bei den anderen mit dabei, weil die sonst auch durch COMMAND_SCHEMAS.ITYPE gematcht werden würden.

@mclrc
Copy link
Owner

mclrc commented Mar 23, 2024

guter punkt, hatte ich ganz vergessen! :)
eine kleine sache noch - ich glaube das ist hier (noch) nicht der fall, aber wenn ITYPE_OPCODES[name] definiert aber 0 wäre, würde trotzdem MEMORY_OPCODES[name] verwendet werden (was dann nicht definiert ist) weil 0 falsy ist. wenn du ein fallback für undefined/null haben willst, ist der ?? operator was du suchst. ist relativ neu aber ein bisschen bugsicherer.

@impaulsible
Copy link
Contributor Author

hab ich gemacht (auch wenn opcode 0 laut green card eigentlich nie vorkommen sollte)

@mclrc
Copy link
Owner

mclrc commented Mar 25, 2024

top, vielen dank! :) da hast du glaube ich recht, aber ich würde || für diese Anwendung generell einfach vermeiden da es schon zu verwirrenden problemen führen kann

@mclrc mclrc merged commit 52711cd into mclrc:main Mar 25, 2024
7 checks passed
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