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

Tools: Add IDA Save/Load Dolphin Map Scripts for 7.x #10157

Merged
merged 1 commit into from Oct 12, 2021

Conversation

dreamsyntax
Copy link
Contributor

@dreamsyntax dreamsyntax commented Oct 9, 2021

IDA 7.4+ breaks compatibility with IDA 6.x style scripts.
All IDA 7.0+ versions are compatible with IDA 7.x style scripts.

Updated unsupported calls per official hex-rays document:
https://hex-rays.com/products/ida/support/ida74_idapython_no_bc695_porting_guide.shtml

  • Kept legacy scripts and made folders IDA 6.X and IDA 7.X in Tools/IDA - intentionally specifying its the program again rather than allowing assumption of script version.
  • Added cancel check to save/load for all scripts (including 6.x scripts)
  • Updated copyright and license info to reflect current status

@mbc07
Copy link
Contributor

mbc07 commented Oct 9, 2021

I might be wrong but AFAIK the convention of this repository is to not change the copyright year of existing files...

@dreamsyntax
Copy link
Contributor Author

I might be wrong but AFAIK the convention of this repository is to not change the copyright year of existing files...

All I could find is
https://github.com/dolphin-emu/dolphin/blob/master/Contributing.md#dolphin-coding-style--licensing

Which does not specify this. If someone can confirm I'll change the 6.x scripts, though they were modified/changed with the extra check.

@JosJuice
Copy link
Member

JosJuice commented Oct 9, 2021

It is our policy that the copyright year is based on when the file was created, not when it was last modified. Though indeed, I don't think I can see any place where this policy is written down other than review comments on PRs.

@Pokechu22
Copy link
Contributor

It's mentioned at #1826 (comment) and #7786 (comment) for instance.

@phire
Copy link
Member

phire commented Oct 9, 2021

Theoretically it could be updated if the file has substantially changed, but you have only made minor changes to those files here.

@dreamsyntax
Copy link
Contributor Author

dreamsyntax commented Oct 9, 2021

It is our policy that the copyright year is based on when the file was created, not when it was last modified. Though indeed, I don't think I can see any place where this policy is written down other than review comments on PRs.

Gotcha, will update.

Theoretically it could be updated if the file has substantially changed, but you have only made minor changes to those files here.

For clarity sake, since the original files were moved those should stay as 2018.
The 'new' files (which are just copies + sdk/func call changes) should also be 2018? Or should those be 2021 since they did not exist prior?

I'm just opting to set them all to 2018 because the changes are minor, even to the 7.x change. If it was a refactor/written from scratch I think 2021 would be justified. Re-pushed with 2018 on all.

@JMC47
Copy link
Contributor

JMC47 commented Oct 9, 2021

@dolphin-emu-bot rebuild

@JosJuice
Copy link
Member

JosJuice commented Oct 9, 2021

The 'new' files (which are just copies + sdk/func call changes) should also be 2018? Or should those be 2021 since they did not exist prior?

I would've set them to 2021, but I don't think it really matters. It's a bit arbitrary. Let's leave it as it is unless someone else objects.

@phire
Copy link
Member

phire commented Oct 9, 2021

New files should be 2021.

@dreamsyntax
Copy link
Contributor Author

The 'new' files (which are just copies + sdk/func call changes) should also be 2018? Or should those be 2021 since they did not exist prior?

I would've set them to 2021, but I don't think it really matters. It's a bit arbitrary. Let's leave it as it is unless someone else objects.

New files should be 2021.

Changed the 7.x files to 2021, per objection. pushed.

@MasterofGalaxies
Copy link

Looks Tools/ was missed in #9862. The license comments should be SPDX tags, no?

@JosJuice
Copy link
Member

JosJuice commented Oct 9, 2021

Yes, ideally. But since it was missed in that PR, I'm fine with leaving it as it is in this PR, so as to not put a bunch of unrelated changes in the same PR.

@leoetlino leoetlino merged commit cac74f0 into dolphin-emu:master Oct 12, 2021
10 checks passed
@dreamsyntax dreamsyntax deleted the ida-7.x-scripts branch October 13, 2021 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants