Skip to content

Conversation

@rikardn
Copy link
Contributor

@rikardn rikardn commented Oct 19, 2025

  • An update of mov_FAC1_FAC2 and mov_r_FAC1_FAC2 to match the Microsoft implementation.
  • Added the Microsoft license to LICENSE
  • Removed jump because it now fits in the slot
  • Added links to the original code in each file
  • Added notes that this is identical to the original implementation in each file

@rikardn
Copy link
Contributor Author

rikardn commented Oct 19, 2025

Perhaps I should switch to using a local label for the loop -> Now updated

@rikardn rikardn force-pushed the update_mov_FAC1_FAC2 branch from 9933585 to 4e235eb Compare October 19, 2025 14:00
@FeralChild64
Copy link
Collaborator

  1. Could you also update the https://github.com/MEGA65/open-roms/blob/master/STATUS.md? Set the Status of these routines to DONE, and put a remark like "Taken from the Microsoft BASIC, original name: MOVEAF".

  2. What about he similarity detection? We need some way to prevent/silence the similarities affecting the open-source Microsoft code.

;
; Math package - round and move FAC1 to FAC2
;
; This is identical to the original Microsoft implementation where it was named MOVAF.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We definitely need to mention that this file is covered by the MIT License. And preferably just below the ;; #LAYOUT# metadata, not later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I'll fix

@rikardn
Copy link
Contributor Author

rikardn commented Oct 19, 2025

I can update the STATUS. I agree that we should turn off the similarity detection of all areas of the BASIC. I can look into it.

@rikardn rikardn force-pushed the update_mov_FAC1_FAC2 branch from 4e235eb to d375b81 Compare October 19, 2025 18:50
@rikardn
Copy link
Contributor Author

rikardn commented Oct 19, 2025

I have updated the PR with the above.

Similarity testing for the part of the basic that is in the kernal ROM remains. Currently it doesn't fail, but we will have to remove it at some point. Is it ok to leave it out from this PR?

testsimilarity: $(TOOL_SIMILARITY) $(DIR_GEN)/OUTx_x.BIN $(ROM_CBM_KERNAL) $(ROM_CBM_BASIC)
testsimilarity: $(TOOL_SIMILARITY) $(DIR_GEN)/OUTx_x.BIN $(ROM_CBM_KERNAL)
$(TOOL_SIMILARITY) $(ROM_CBM_KERNAL) $(DIR_GEN)/OUTx_x.BIN
$(TOOL_SIMILARITY) $(ROM_CBM_BASIC) $(DIR_GEN)/OUTx_x.BIN
Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't mean completely removing the BASIC ROM similarity checks - Commodore might have made some improvements there, we still need the check.

What I meant was to be able to skip checking individual procedures, like $BC0C-$BC1A area. My proposal: mask this area with 0's, either using some script here (so that the similarity tool would receive an altered ROM images), or within the similarity tool itself.

;; #LAYOUT# * BASIC_0 #TAKE
;; #LAYOUT# * * #IGNORE

; This file is under the MIT license. See LICENSE for more information.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should also mention the Microsoft Corporation here. MIT licenses might have slightly different copyright notices, crediting different authors - we need to be more specific.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this:

; This file is under the MIT license, it contains code released by Microsoft Corporation.
; See LICENSE for more information.

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