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

adding command line support #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

unsupo
Copy link

@unsupo unsupo commented Jun 29, 2023

This is a starting point to show what a command line utility would look like.
The original code still works the same in a browser, but now will also work as a command line utility.

example command
node index.js patch "GBA FILE.gba" "PATCH FILE.ips"

I did have to duplicate a lot of the code in RomPatcher.js, but just in case i decide not to clean that up I opened up this PR. This is a working tool.

@marcrobledo
Copy link
Owner

Looks neat, thank you for your contribution!
Need to find some time to take an in depth look at the changes though, as I've barely touched Node.js!

@unsupo
Copy link
Author

unsupo commented Jun 30, 2023

if this approach is ok with you i don't mind cleaning it up so you can merge. Otherwise we can leave it open for people who want the option until you have an approved approach.

@marcrobledo
Copy link
Owner

if this approach is ok with you i don't mind cleaning it up so you can merge. Otherwise we can leave it open for people who want the option until you have a better approach.

I figure it's possible to avoid repeating code from RomPatcher.js. I'll try to find some time and try to do a clean up of my old (but functional at least!) messy code, the common code must definitively be in another file.

@marcrobledo
Copy link
Owner

I started looking at this at last :-)

@OilSubjectLoss7
Copy link

i'm getting this error

/rompatcherjs_with_cli/RomPatcher.js/js/cmd.js:150
        for(let i=0; i<HEADERS_INFO.length; i++){
                       ^

ReferenceError: HEADERS_INFO is not   ##defined

@marcrobledo
Copy link
Owner

Believe it or not, I'm still working on this.

This forced me to do the code overhaul I always wanted to do for this project :-) It was functional, but I never was satisfied on how the code ended.
There will be some refactor here and there from your commit @unsupo , since I didn't like so much copy duplicated for both web and Node versions.

@CEnnis91
Copy link

CEnnis91 commented Mar 31, 2024

👀

Developer of the Smash Remix patcher here. It's been a while since I worked with this, but I'm very excited to see effort into making this more portable and separate from the UI.

That was one of the things I needed to work around when adapting the patcher for the Smash 64 community.

I would love to help here if possible. I was already thinking about this kind of patch as I was detaching the UI logic in my implementation, but I don't want to cross wires submitting something already in progress.

I'm curious of the thoughts of @marcrobledo and @unsupo to creating a more shared space to discuss this. Naturally Marc would have the final say, but I would still like to hear unsupo's thoughts. I'm partial to something like Discord or Matrix, but flexible.

If we don't like that idea, maybe @marcrobledo can share the idea of the code overhaul in progress?

@unsupo
Copy link
Author

unsupo commented Apr 1, 2024

@CEnnis91 @marcrobledo thanks for including me. I'm not sure if i should have much say in this conversation. @CEnnis91 if you have something better i'm more than happy to let @marcrobledo close this and use your implementation. This was a while ago and I frankly forgot about this after i used it for what i needed. Let me know if you need any else from me.

@marcrobledo
Copy link
Owner

👀

Developer of the Smash Remix patcher here. It's been a while since I worked with this, but I'm very excited to see effort into making this more portable and separate from the UI.

Wow! Thank you for your hack, I always wait impatiently for the new updates and characters ^_^
Thank you for using my Rom Patcher as well in your website.

Indeed, seeing your website using Rom Patcher JS was the boost to force myself to work on an overhaul, as I saw you had to mess and do lots of customizations. That's my aim now: to allow anyone to embed the patcher easily.

detaching the UI logic in my implementation

That's what I am doing :-)
But I also wanted to implement new small features like drag and drop and other improvements.

I'm curious of the thoughts of @marcrobledo and @unsupo to creating a more shared space to discuss this. Naturally Marc would have the final say, but I would still like to hear unsupo's thoughts. I'm partial to something like Discord or Matrix, but flexible.

I think the best I can do first is to merge @unsupo 's PR at last...

If we don't like that idea, maybe @marcrobledo can share the idea of the code overhaul in progress?

...then start a branch with the new code.

It will be great to have some help. I tend to do my personal projects all by myself. But it is a reality that I have no time for all of them.

Sure, we can discuss it in Discord if you want!

@marcrobledo
Copy link
Owner

I'll do a code cleanup this weekend and create the branch!

@besteon
Copy link

besteon commented Apr 18, 2024

I'll do a code cleanup this weekend and create the branch!

Hey there, dev of Ironmon-Tracker here. I am also invested in getting a CLI version of this tool so I can make it easier to add the quality of life patches that our community uses. Please let me know if you need any assistance with the branch!

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.

None yet

6 participants