Feature/functional opcodes #11
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
opposed to them just being data that the CPU acts on the existence of. It get's rid of the huge case statement but doesn't really simplify anything but it's worth the excercise.
This idea splits opcodes into three levels:
- the operation (e.g. ASL) which includes everything needing done to
execute an operation
- the addressing mode (ACCUMULATOR), including getting an setting
values from an addressed location
- the combination of both to allow us to say things like ASL_A.execute()
OpCode already knows what it's AddressingMode and Operation are, so explicitly stating them in a passed in function for each was overkill. All I needed to do was call whatever AddressingMode and Operation was part of that OpCode. This is the same for all OpCodes so one method for all replaces one for each.
They were in the wrong order
Including the addressing modes needed by LDX. - Immediate - Zero Page Y - Absolute Y
TODO: Review how BIT works.
Also : Enabled BIT tests that I forgot to enable
- Renamed operation to have 6502 prefix - Renamed addressing mode to have 6502 prefix
BPL,BMI,BVC,BVS,BNE,BEQ
Could do with tidying up the stack logic
operation Javadoc Now that all opcodes than can be enum-ed, have been, we can get rid of the specific cases and apply the default to all but the exceptions. Would still be nice to come up with a JMP strategy that allows us to get rid of the case entirely but for now, it's much neater.
- More appropriate access modifiers - map.putAll replaced with initialisation - Collapsing exception catches that are identical - Needless functional interface method reference - Unused imports - Improved Javadoc
Codecov Report
@@ Coverage Diff @@
## master #11 +/- ##
==========================================
- Coverage 98.54% 97.54% -1%
+ Complexity 516 413 -103
==========================================
Files 24 25 +1
Lines 1304 1305 +1
Branches 97 91 -6
==========================================
- Hits 1285 1273 -12
- Misses 10 20 +10
- Partials 9 12 +3
Continue to review full report at Codecov.
|
|
Next step should be to fix the test coverage |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Been looking at getting rid of the huge switch statement and mass of partner methods in the 6502. Largely this method has solved that problem and broken it down. This doesn't work so well for JMP commands but cleans the code up and makes it much more flexible.
This ends up saving about 200 loc and adding to the readability and composability. Also flagged up a few issues with my first implementation so it was a good exercise.