korcankaraokcu / PINCE

Reverse engineering tool for linux games
Other
2.13k stars 149 forks source link

Feature Request: Not option when "exact scanning" #253

Closed OpenNetSurfer closed 6 months ago

OpenNetSurfer commented 6 months ago

A small request Like in cheat engine, add an Not option when exact scan is selected. This will remove results matching the given value.

Edit1->2: Also the Not option should be disabled with BYTEARRAY and 'STRING' selected

OpenNetSurfer commented 6 months ago

I have created a fork for anyone to view I am doing Currently changed GUI/MainWindow.py and PINCE.py to simply show an option

https://github.com/OpenNetSurfer/PINCE

korcankaraokcu commented 6 months ago

You've changed an auto-generated file without using the Qt Designer. Your changes will be completely erased within the next patch that modifies the MainWindow ui file. Check the guidelines before contributing

OpenNetSurfer commented 6 months ago

Yes I did read the guidelines before forking, but I am finding a little difficulty installing Qt Designer. For now I will manually change the MainWindow.py a little so that I can get back to using MainWindow.ui.

Could you suggest a forum or a wiki to install and use? It will be helpful.

Edit: Typo fix

korcankaraokcu commented 6 months ago

You need to have Qt6 Designer and pyuic6 installed. If there are no available packages for your distro, install pyqt6-tools instead

pyqt6-tools is on PyPI, can be easily installed via a single command pip install pyqt6-tools

OpenNetSurfer commented 6 months ago

Thanks for help. I was using apt instead of pip unfortunately -_-

OpenNetSurfer commented 6 months ago

I have added the feature! Go to https://github.com/OpenNetSurfer/PINCE to see. Only works with EXACT scan, and not with AOB or STRING scan type though.

I will now ready the repo for a pull request. Feel free to review it. Should pull request in 24 hours, as this is my first time pull requesting, want to be completely sure of it.

Bloodiko commented 6 months ago

If the "not" option only works with "exact" Scan Type anyway, whats the reasoning behind using a checkbox ?

I think using "NOT" as a separate SCAN_TYPE, next to EXACT / CHANGED / UNCHANGED seems more logical. Since you can only choose one scan type anyway, why not adding "NOT" to the dropdown as a separate scan_type.

(AOB and STRING will fail with any scan type other than EXACT anyway, may need a bugfix for that probably anyway.)

OpenNetSurfer commented 6 months ago

I added Not as an checkbox because that's how cheat engine also uses. Also in future I have plans on using "not" option in other scan types too. Like in Between, Increased by, Decreased by. Scanmem holds some functionality to do something like that so I thought of adding as a checkbox for future uses rather than a scan type option.

brkzlr commented 6 months ago

A couple of things: 1) If the PR won't contain those new capabilities you speak off, please keep it consistent with the codebase and simpler. Move the Not into it's own scantype and get rid of the checkbox. This way it closely emulates libscanmem's operator function behaviour. 2) Please make your changes in as few commits as possible in a new branch and then PR off that branch. There's no need for junk commits such as README.md modifications stating it's a fork and reverting it after, or multiple libscanmem submodule pointing changes that will be reverted anyway.

Golden rule is to have each commit be a self-contained logical change that does not break the program in any way. This way the PR should also be more readable than seeing x amount of commits that do not contribute anything with (Please ignore this).

All you need for this functionality is just one commit that adds typedefs.SCAN_TYPE.NOT which sends "!=" to libscanmem.

OpenNetSurfer commented 6 months ago

I see. I will implement it as an scan type. I am sorry for those useless commits, I am just getting started to using git.

brkzlr commented 6 months ago

If you have Discord, feel free to join the server in case you might need more pointers about these. I wouldn't recommend opening an issue just to discuss possible PRs/features.

Also no need to apologize for the commits. It's your fork, you're free to do whatever you want in there. My point was that make sure to not include these in the PR as they're not helpful for the PR/feature itself.

OpenNetSurfer commented 6 months ago

I have a question. Is this good implementation of "Not" option? Any comments?

Context: If AOB or STRING as Value_Type is selected, disable "Not" option as a SCAN_TYPE. This function below will be called always from comboBox_ValueType_current_index_changed().

    def comboBox_ScanType_init(self):
        scan_type_text = {
            typedefs.SCAN_TYPE.EXACT: tr.EXACT,
+           typedefs.SCAN_TYPE.NOT: tr.NOT,
            typedefs.SCAN_TYPE.INCREASED: tr.INCREASED,
            typedefs.SCAN_TYPE.INCREASED_BY: tr.INCREASED_BY,
            typedefs.SCAN_TYPE.DECREASED: tr.DECREASED,
            typedefs.SCAN_TYPE.DECREASED_BY: tr.DECREASED_BY,
            typedefs.SCAN_TYPE.LESS: tr.LESS_THAN,
            typedefs.SCAN_TYPE.MORE: tr.MORE_THAN,
            typedefs.SCAN_TYPE.BETWEEN: tr.BETWEEN,
            typedefs.SCAN_TYPE.CHANGED: tr.CHANGED,
            typedefs.SCAN_TYPE.UNCHANGED: tr.UNCHANGED,
            typedefs.SCAN_TYPE.UNKNOWN: tr.UNKNOWN_VALUE,
        }
        current_type = self.comboBox_ScanType.currentData(Qt.ItemDataRole.UserRole)
        self.comboBox_ScanType.clear()
        items = typedefs.SCAN_TYPE.get_list(self.scan_mode)
+
+       # Delete Not option if Value_Type selected is AOB or STRING
+       if self.comboBox_ValueType.currentData(Qt.ItemDataRole.UserRole) == typedefs.SCAN_INDEX.AOB or self.comboBox_ValueType.currentData(Qt.ItemDataRole.UserRole) == typedefs.SCAN_INDEX.STRING:
+           del items[1]    # Corresponds to NOT in "get_list()" return
+
        old_index = 0
        for index, type_index in enumerate(items):
            if current_type == type_index:
                old_index = index
            self.comboBox_ScanType.addItem(scan_type_text[type_index], type_index)
        self.comboBox_ScanType.setCurrentIndex(old_index)

Or should I change somethings in the get_list() function?

OpenNetSurfer commented 6 months ago

I have made required changes to the repo. I will make a pull request in a few hours. Please review it if you can: https://github.com/OpenNetSurfer/PINCE

Changed files:

  1. PINCE.py
  2. tr/tr.py
  3. i18n/ts/it_IT.ts (using compile_ts.sh)
  4. i18n/ts/zh_CN.ts (using compile_ts.sh)
  5. libpince/typedefs.py
brkzlr commented 6 months ago

Reviews are made in PRs not issue threads.

Closing this issue since you already worked on the feature.