gerwaric / acquisition

Stash and forum shop management for Path of Exile
GNU General Public License v3.0
18 stars 3 forks source link

Application crashes with v0.11.0 #39

Closed aiolos01 closed 3 months ago

aiolos01 commented 5 months ago

Several bugs to report. First of all I get a dialog saying the latest version is 0.11 despite already using 0.11

Second it crashes every time I choose refresh checked or refresh all tabs. If I select some tabs and choose refresh selected it doesn't crash.

Third, I got a different crash once. Snap247

All this if I use OATH. If I use POESESSID there are no crashes.

gerwaric commented 5 months ago

Thank you for the reports. I'll put together a manual release checklist for future updates, since I don't know how to automate UI testing. A lot changed under the hood witih this release, so I'm glad at least POESESSID still works for you.

I hope those issue don't take long to fix, because I really would like to fix pseudo-mod searching next.

gerwaric commented 4 months ago

@aiolos01 When you have time, I have an update that should fix most of the bugs you found:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.1

I was able to run all three kinds of refreshes on both POESESSID and OAuth without crashes, with a random variety of tabs checked. I also think I fixed the update notice logic. Finally, I've added extra error logging in places where I think I might be causing a crash, e.g. before dereferencing a pointer that might be null.

The one issue I haven't fixed is the bucket row out of bounds error, because I haven't been able to replicate it yet. Are there certain actions you take that reliabley cause that error?

Thanks for your patience.

aiolos01 commented 4 months ago

Hi, I've run it twice.Once refreshed all checked and once refreshed all tabs. Both cause immediate crashes. I can't see anything useful in the log. Is there some other log?

INFO 2024-07-03T01:18:01.246

INFO 2024-07-03T01:18:01.247 acquisition 0.11.0.0 ( version code 61 ) INFO 2024-07-03T01:18:01.247 Built with Qt 6.5.3 on Wed Jun 19 21:10:30 2024 INFO 2024-07-03T01:18:01.247 Running on Qt 6.5.3 INFO 2024-07-03T01:18:01.247 Logging level is INFO INFO 2024-07-03T01:18:01.315 Running application... INFO 2024-07-03T01:18:01.383 OAuth: refreshing token at "Mon Jun 24 11:33:57 2024 GMT+0300" INFO 2024-07-05T18:17:04.748

INFO 2024-07-05T18:17:04.751 acquisition 0.11.1.0 ( version code 61 ) INFO 2024-07-05T18:17:04.751 Built with Qt 6.5.3 on Mon Jul 1 22:12:12 2024 INFO 2024-07-05T18:17:04.751 Running on Qt 6.5.3 INFO 2024-07-05T18:17:04.751 Logging level is INFO INFO 2024-07-05T18:17:04.753 Running application... INFO 2024-07-05T18:17:04.764 Removing the stored OAuth token because it has expired. WARN 2024-07-05T18:17:28.579 Constructing OAuth token without a birthday. WARN 2024-07-05T18:17:28.579 Constructing OAuth token without an expiration INFO 2024-07-05T18:17:28.594 OAuth: refreshing token at "Sat Jul 6 03:17:28 2024" INFO 2024-07-05T18:17:36.900 Starting OAuth authentication INFO 2024-07-05T18:17:37.682 I've created the folder "....." INFO 2024-07-05T18:17:37.796 Logging level set to "INFO" INFO 2024-07-05T18:17:38.176 Loading item classes from RePoE. INFO 2024-07-05T18:17:38.652 Loading item base types from RePoE. INFO 2024-07-05T18:17:39.944 There are 6629 items and 203 tabs after the refresh. INFO 2024-07-05T18:17:39.944 There are 33 uncategorized items. INFO 2024-07-05T18:19:51.672

INFO 2024-07-05T18:19:51.683 acquisition 0.11.1.0 ( version code 61 ) INFO 2024-07-05T18:19:51.683 Built with Qt 6.5.3 on Mon Jul 1 22:12:12 2024 INFO 2024-07-05T18:19:51.683 Running on Qt 6.5.3 INFO 2024-07-05T18:19:51.683 Logging level is INFO INFO 2024-07-05T18:19:51.685 Running application... INFO 2024-07-05T18:19:51.693 OAuth: refreshing token at "Sat Jul 6 03:17:28 2024 GMT" INFO 2024-07-05T18:19:53.925 Starting OAuth authentication INFO 2024-07-05T18:19:54.661 Logging level set to "INFO" INFO 2024-07-05T18:19:54.842 Loading item classes from RePoE. INFO 2024-07-05T18:19:55.024 Loading item base types from RePoE. INFO 2024-07-05T18:19:55.672 There are 6629 items and 203 tabs after the refresh. INFO 2024-07-05T18:19:55.672 There are 33 uncategorized items.

On Tue, Jul 2, 2024 at 7:25 AM GERWARIC @.***> wrote:

@aiolos01 https://github.com/aiolos01 When you have time, I have an update that should fix most of the bugs you found:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.1

I was able to run all three kinds of refreshes on both POESESSID and OAuth without crashes, with a random variety of tabs checked. I also think I fixed the update notice logic. Finally, I've added extra error logging in places where I think I might be causing a crash, e.g. before dereferencing a pointer that might be null.

The one issue I haven't fixed is the bucket row out of bounds error, because I haven't been able to replicate it yet. Are there certain actions you take that reliabley cause that error?

Thanks for your patience.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2201879351, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZL5AONPQAIEM6WZFQDZKITULAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBRHA3TSMZVGE . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

Heck. I thought I had guarded the most likely failures with error logging, but I clearly missed something. Could you turn up the logging to TRACE level and run one more time?

There is a submenu in the Setting menu to set the logging level. I'm hoping the additional logging will give a few more clues. In the meantime, I'll work on something like a debug mode to track exactly what's happening on a function-by-function basis.

(Acquisition also used to provide stack dumps when it crashed, but that was years ago. I'll look into this as well, since that could tell us exactly where the crash happened).

gerwaric commented 4 months ago

UPDATE: I think I have figured out how to use Windows crash dumps, which will let me see exactly where acquisition crashed for you. This would mean no more trying to guess from the log files. If we can get it working, I'll know exactly which line in which function is causing the crash.

Are you open to chatting on Discord? I'm gerwaric there as well. There are some tools to automate crash reporting, but they would take time for me to figure out, integrate into acquisiton, and test. It will be faster and easier to start with Discord.

aiolos01 commented 4 months ago

I don't see anything useful in the log with trace on either.

INFO 2024-07-07T19:11:20.807 Logging level set to "TRACE" DEBUG 2024-07-07T19:11:33.527 Updating Checked stash tabs DEBUG 2024-07-07T19:11:33.527 Updating 89 tabs. DEBUG 2024-07-07T19:11:33.527 Keeping 114 tabs and culling 89 DEBUG 2024-07-07T19:11:33.527 Keeping 0 items and culling 6585 DEBUG 2024-07-07T19:11:33.765 Creating rate limit policy "stash-list-request-limit" for "GET /stash/" DEBUG 2024-07-07T19:11:33.765 "stash-list-request-limit" increasing history capacity from 0 to 30 TRACE 2024-07-07T19:11:33.765 RateLimiter is PAUSED 0 for "" TRACE 2024-07-07T19:11:33.765 "stash-list-request-limit" waiting 0 seconds to send request 1 at "Sun Jul 7 19:11:33 2024" TRACE 2024-07-07T19:11:33.765 RateLimiter is PAUSED 0 for "stash-list-request-limit" TRACE 2024-07-07T19:11:34.017 "stash-list-request-limit" sending request 1 to "GET /stash/" via "https://api.pathofexile.com/stash/Standard" TRACE 2024-07-07T19:11:34.259 "stash-list-request-limit" received reply for request 1 with status 200 TRACE 2024-07-07T19:11:34.259 Updating next send: from "Sun Jul 7 19:11:33 2024" to "Sun Jul 7 19:11:34 2024" TRACE 2024-07-07T19:11:34.260 RateLimiter is PAUSED 0 for "stash-list-request-limit" TRACE 2024-07-07T19:11:34.260 OAuth stash list received DEBUG 2024-07-07T19:11:34.260 Received stash list, there are 161 stash tabs DEBUG 2024-07-07T19:11:34.260 Queued ( 1 ) -- "#1, \"...\"" DEBUG 2024-07-07T19:11:34.260 Queued ( 2 ) -- "#2, \" ... \"" DEBUG 2024-07-07T19:11:34.260 Queued ( 3 ) -- "#3, \" ... \"" DEBUG 2024-07-07T19:11:34.260 Queued ( 4 ) -- "#4, \" ... \"" DEBUG 2024-07-07T19:11:34.260 Queued ( 5 ) -- "#5, \" ... \"" DEBUG 2024-07-07T19:11:34.260 Queued ( 6 ) -- "#6, \" ... \"" DEBUG 2024-07-07T19:11:34.260 Queued ( 7 ) -- "#7, \" ... \"" DEBUG 2024-07-07T19:11:34.260 Queued ( 8 ) -- "#8, \" ... \""

It seems to crash after queuing the currency tab. I don't know if that helps in some way. I have an account on discord, not that I ever use it. I'll try to PM you there.

On Fri, Jul 5, 2024 at 10:13 PM GERWARIC @.***> wrote:

UPDATE: I think I have figured out how to use Windows crash dumps, which will let me see exactly where acquisition crashed for you. This would mean no more trying to guess from the log files. If we can get it working, I'll know exactly which line in which function is causing the crash.

Are you open to chatting on Discord? I'm gerwaric there as well. There are some tools to automate crash reporting, but they would take time for me to figure out, integrate into acquisiton, and test. It will be faster and easier to start with Discord.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2211308819, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZI75MFSIA5DYE534JDZK3V6VAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJRGMYDQOBRHE . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

Over the weekend I found a way to automatically report crashes:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.2

I've tested it on Windows on my laptop. On the crash reporting website, I can see the file and line number of each crash, although there is no account information, so I won't know which crash came from which user without trying to compare upload times with log messages people report here.

If you can give it a try, I would love to confirm that it works for someone else.

aiolos01 commented 4 months ago

OK I've run it. This one crashes on launch. I assume crashpad is the utility that sends the report, I hope it helps.

On Mon, Jul 8, 2024 at 3:48 AM GERWARIC @.***> wrote:

Over the weekend I figured out an approach to automatic crash reporting:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.2

I've testing it on Windows on my machine. I can see the file and line number of each crash, although there is no account information, so I won't know which crash came from which user without trying to compare upload times with log messages people report here.

If you can give it a try, I would love to confirm that this works for someone else.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2212688996, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZIKJQBZMXWAPP2WMVDZLHOXTAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJSGY4DQOJZGY . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

Yes, acquisition is using crashpad to send reports to BugSplat, where I'm hoping to get by with the free tier. Unfortunately that didn't help, however. The crash reporting isn't configured correctly yet.

Apologies this taking so long. I'd love to get OAuth working, but this is frustrating.

When you can get to it, here's a special release that is a zip file instead of an installer. You can just run it from where you unzip it:

~https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.trace.3~ https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha-trace.4 (even more logging than trace.3)

That build will default to the TRACE log level. That's going to flood the log file with messages. I've added a lot of extra log messages at that TRACE level now, and I'm working on more. So far it's mostly focused on the code that executes before the login dialog is shown, but I want it to cover the entire codebase to help with debugging in the future.

If you could run the latest alpha-trace build and post the last 5 lines from your log file after a crash, I'm hoping this will narrow it down. Once this issue is fixed, I'll go back to building installers and setting the default logging level to INFO like it was before.

aiolos01 commented 4 months ago

I believe I've sent a couple of reports with the new version

On Tue, Jul 9, 2024 at 3:41 AM GERWARIC @.***> wrote:

Unfortunately that didn't help. I don't have the crash reporting configured correctly yet. Apologies for taking so many iterations on this. I would really like to get the OAuth code working, but this is frustrating.

When you can get to it, here's a special release that is a zip file instead of an installer. You can just run it from where you unzip it: https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.trace.3

That build will default to the TRACE log level, and I've added a lot of extra log messages at this level to most of the code that executes before the login dialog is shown. If you could run it and post the last 5 lines from your log file, I'm hoping this will narrow it down.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2215811366, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZID7ZW2IH2PIOE5NC3ZLMWSRAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJVHAYTCMZWGY . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

Yes. I can see a crash with alpha-trace.3 that looks plausible. This may have sold me on BugSplat, because it would have taken a long time to find the issue without it.

Specifically, the crash occured while trying to parse the colour of a stash tab after receiving the list of stashes in OAuth mode. That field is optional according to the GGG API developer documentation. My OAuth code assumed it was always present. This wasn't happening to me, so apparently all my tabs have colour.

If you were experiencing this in Standard, it's possible that stash tabs created in the early days of PoE don't have that colour field. This problem is also a non-issue when using POESESSID because the legacy API returns stash tabs in a different json format.

Long story short--this should fix that particular crash: https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-colour-fix

While you're testing that, I've been down in the guts of the items model trying to diagnose the "bucket row out of bounds error." It's making my head hurt, but that's the next thing I'm working on.

aiolos01 commented 4 months ago

Yes I'm on standard. Unfortunately this still crashes in both SSID and OATH login options. Sent 2 reports.

On Wed, Jul 10, 2024 at 9:29 PM GERWARIC @.***> wrote:

Yes. I can see a crash with alpha-trace.3 that looks plausible. This may have sold me on BugSplat, because it would have taken a long time to find the issue without it.

Specifically, the crash occured while trying to parse the colour of a stash tab after receiving the list of stashes in OAuth mode. That field is optional according to the GGG API developer documentation. My OAuth code assumed it was always present. This wasn't happening to me, so apparently all my tabs have colour.

If you were experiencing this in Standard, it's possible that stash tabs created in the early days of PoE don't have that colour field. This problem is also a non-issue when using POESESSID because the legacy API returns stash tabs in a different json format.

Long story short--this should fix that particular crash: https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-colour-fix

While you're testing that, I've been down in the guts of the items model trying to diagnose the "bucket row out of bounds error." It's making my head hurt, but that's the next thing I'm working on.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2221175295, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZOCVAK62M3MWLZGFOLZLV4QZAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRRGE3TKMRZGU . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

One of those crashes is showing up on BugSplat, and it looks the crash was outside acquisition in the Microsoft Visual C++ runtime library msvcp140.dll:

d:\a01\_work\12\s\src\vctools\crt\github\stl\src\mutex.cpp(103):
mtx_do_lock(_Mtx_internal_imp_t*, xtime const*)

Can you check to see which version(s) of the c++ runtime you have installed?

Here's what's on my development system: image

aiolos01 commented 4 months ago

The latest installed is 14.36.33130. Inside the acquisition install folder are: msvcp140.dll 14.29.30139.0 msvcp140_1.dll 14.29.30139.0 msvcp140_2.dll 14.29.30139.0

On Thu, Jul 11, 2024 at 6:04 PM GERWARIC @.***> wrote:

One of those crashes is showing up on BugSplat, and it looks the crash was outside acquisition in the Microsoft Visual C++ runtime library msvcp140.dll:

d:\a01_work\12\s\src\vctools\crt\github\stl\src\mutex.cpp(103): mtx_do_lock(_Mtx_internal_imp_t, xtime const)

Can you check to see which version(s) of the c++ runtime you have installed?

Here's what's on my development system: image.png (view on web) https://github.com/gerwaric/acquisition/assets/6461624/5304fcf6-015f-4a79-a5c9-dd0ac20826b4

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2223175180, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZIFBYUFBBDMVRZXAZLZL2NHFAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRTGE3TKMJYGA . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

I'll see if I can roll back to those versions and replicate the bugs you're seeing. If that works, then I can link to the latest installer from Microsoft and mention it in the release notes. I'll update you here.

gerwaric commented 4 months ago

Please delete those files from the installation folder (msvcp140.dll, msvcp140_1.dll, and msvcp140_2.dll) and let me know what happens.

I installed multiple versions of the visual c++ runtime one at a time, from 14.20.27519 to 14.40.33810, including 14.29.30139. Each time I tried running acquisition both with and without those three files from 14.29.30139, and acquisition crashed every time those .dlls were present in the installation folder.

aiolos01 commented 4 months ago

I deleted them and now I can run acquisition. With SSID I can update the tabs. With OATH it crashes when I try to update (tried update all). Report sent.

On Fri, Jul 12, 2024 at 2:02 AM GERWARIC @.***> wrote:

Please delete those files from the installation folder (msvcp140.dll, msvcp140_1.dll, and msvcp140_2.dll) and let me know what happens.

I installed multiple versions of the visual c++ runtime one at a time, from 14.20.27519 to 14.40.33810, including 14.29.30139. Each time I tried running acquisition both with and without those three files from 14.29.30139, and acquisition crashed every time those .dlls were present in the installation folder.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2224091733, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZODAPOWV7FUOQKXMPTZL4FKDAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRUGA4TCNZTGM . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

Whew. I'm glad that SSID works for you again. I'll see what I can figure out from those crash reports and try to get another alpha release soon.

gerwaric commented 4 months ago

The next release has extra checks when parsing the OAuth json, so if that was causing the crashes you are seeing, they will at least show up in the log file as warnings or errors.

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.6

This update should also warn you if there are extra dll files in the application folder, since there may be other users experiencing this crash.

aiolos01 commented 4 months ago

Ok this seems to work when updating tabs with OATH. However it crashes in searches with OATH. The same search works with SSID. I find this weird since searching is done locally but I guess it's not as simple as that.

On Tue, Jul 16, 2024 at 1:01 AM GERWARIC @.***> wrote:

The next release has extra checks when parsing the OAuth json, so if that was causing the crashes you are seeing, they will at least show up in the log file as warnings or errors.

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.6

This update should also warn you if there are extra dll files in the application folder, since there may be other users experiencing this crash.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2229512279, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZM7HAPSJFGYFDMOEVLZMRBCVAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRZGUYTEMRXHE . You are receiving this because you were mentioned.Message ID: @.***>

aiolos01 commented 4 months ago

Scratch that. It crashes with SSID too during search.

On Tue, Jul 16, 2024 at 1:56 PM Nikos Papadopoulos @.***> wrote:

Ok this seems to work when updating tabs with OATH. However it crashes in searches with OATH. The same search works with SSID. I find this weird since searching is done locally but I guess it's not as simple as that.

On Tue, Jul 16, 2024 at 1:01 AM GERWARIC @.***> wrote:

The next release has extra checks when parsing the OAuth json, so if that was causing the crashes you are seeing, they will at least show up in the log file as warnings or errors.

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.6

This update should also warn you if there are extra dll files in the application folder, since there may be other users experiencing this crash.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2229512279, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZM7HAPSJFGYFDMOEVLZMRBCVAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRZGUYTEMRXHE . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

Scratch that. It crashes with SSID too during search.

Oof. I'm sorry. It's possible that once you did a refresh with OAuth the data that caused the crash was saved into the data files. This could be the reason you didn't see a crash with SSID at first.

Fortunately I can see those crashes in bugsplat.

When I get up in a couple hours (I'm on US mountain time), I'll dig into those reports. Hopefully they will help identify whatever elusive thing is happening.

Another thought I have is that if it's crashing in offline search for you, then I should be able to replicate the crash if I had your json data. I don't like the idea of asking you to share data files directly, so I might add an "export" function if the crash reports aren't enough.

Can't stress enough how grateful I am at your persistence, since I can't replicate these crashes on my own. Thank you very much.

aiolos01 commented 4 months ago

Just realised the crashes might have been my fault. Acquisition installed a new VC redist version but I didn't reboot because I was busy with other things. I rebooted now and after some light testing it didn't crash. I'll test it some more and get back to you. Thanks for continuing to work on this.

On Tue, Jul 16, 2024 at 2:22 PM GERWARIC @.***> wrote:

Scratch that. It crashes with SSID too during search.

Oof. I'm sorry. It's possible that once you did a refresh with OAuth the data that caused the crash was saved into the data files. This could be the reason you didn't see a crash with SSID at first.

Fortunately I can see those crashes in bugsplat.

When I get up in a couple hours (I'm on US mountain time), I'll dig into those reports. Hopefully they will help identify whatever elusive thing is happening.

Another thought I have is that if it's crashing in offline search for you, then I should be able to replicate the crash if I had your json data. I don't like the idea of asking you to share data files directly, so I might add an "export" function if the crash reports aren't enough.

Can't stress enough how grateful I am at your persistence, since I can't replicate these crashes on my own. Thank you very much.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2230648977, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZP437CAHFHP352BH3LZMT66TAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZQGY2DQOJXG4 . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

I'm glad to hear the reboot helped. From the crash reports, I think we might be back to your "bucket row out of bounds" error. If you can reliably reproduce that error, a short writeup of how would be useful. I've been playing around with searches, but think I've only seen that error once, so I need to do some more poking around.

gerwaric commented 4 months ago

@aiolos01 I think I may have solved the "bucket row out of bounds error":

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.8

The issue I found is that acquisition always assumed that when the selected item changed, it was changing to a new item. However, it is possible that the change meant that no item was selected. When that happens, an internal row index is negative, which caused the error you screenshotted at the start of this thread. Now acquisition checks for that case and should gracefully do nothing.

When you can get around to testing this, let me know if it's working, or if there are any other new issues for you.

gerwaric commented 4 months ago

@aiolos01 I found a new place where the same bucket row error happens. If I missed that one, I likely missed several, so I'm going to go through the code with a fine-toothed comb tomorrow. I'll reply here when I have an update.

gerwaric commented 4 months ago

I have a lot to say about this issue, but I think it might finally be fixed:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.10

First, I was able to replicate a search crash by clicking on the "corrupted" checkbox, then selecting a search result that was not priced, then clicking on the "priced" checkbox. That should cause the item I had selected to be removed from the search results, but instead it was crashing. It turns out I was incorrectly clearing the selection object after the search was updated, so it was making calls with invalid arguments. This is fixed.

Second, I'm not sure that is the only source of those mysterious search crashes. However, the common issue seems to be certain function calls that lead to dereferencing an invalid pointer, which is a big no-no. To forestall any future crashes, I added extra code to make sure those functions aren't called with invalid arguments. Now, acquisition will log and error and continue running.

Finally, I think the guts of the item searching needs to be refactored. In the ten years since acquisition was created, people now have millions of items, which acquisition was not designed to manager. There are new frameworks that may be simpler, faster, and fewer lines of code. This would be a lot of work, so it's on the back burner for now.

aiolos01 commented 4 months ago

I wouldn't worry about speed. I have lots of tabs (ok not as many as people who play every league) and it's very fast. If the old framework has memory issues (or something else that breaks the app) then it should be fixed, otherwise I wouldn't consider it a priority.

On Mon, Jul 22, 2024 at 6:05 PM GERWARIC @.***> wrote:

I have a lot to say about this issue, but I think it might finally be fixed:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.10

First, I was able to replicate a search crash by clicking on the "corrupted" checkbox, then selected a search result that was not priced, then clicking on the "priced" checkbox, which should cause the item I had selected to be removed from the search results. It turns out I was incorrectly clearing the selection object after the search was updated, so it was making calls with invalid arguments. This is fixed.

Second, I'm not sure that is the only source of those mysterious search crashes. However, the common issue seems to be certain function calls that lead to dereference an invalid pointer, which is a big no-no. To forestall any future crashes, I added extra code to make sure those functions aren't called with invalid arguments. Now, acquisition will log and error and continue running.

Finally, I think the guts of the item searching needs to be refactored. In the ten years since acquisition was created, people now have millions of items, which acquisition was not designed to manager. There are new frameworks that may be simpler, faster, and fewer lines of code. This would be a lot of work, so it's on the back burner for now.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2243187450, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZPVSAGOO5KHX7FFC7TZNUNSTAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBTGE4DONBVGA . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

That's good feedback. Since acquisition isn't popular these days, the only feedback I get about what features people use or care about comes from the ones willing to use GitHub to post issues.

Part of me wants to rebuild a new acquisition from scratch given changes GGG has made, and changes in technology over the last decade. The current guts are pretty complicated and tricky to understand. However, a more practical priority is to improve the mod searching, since it doesn't distinguish between implicit, explicit, and other modifier types the way that the trade site does. It also doesn't compute pseudo-mods at all. (It looks like it might have worked at some point in the past, but right now it's not).

aiolos01 commented 4 months ago

To be honest I don't use acquisition to list items any more. The premium tabs are so easy to use. You don't have to type the name of the item when you get the PM, the tab is listed in the message and the item is even highlighted in the tab. There no way to beat that. It's great that acquisition can read the prices though. It's still useful to see them all in one place without having to hover over each item. The only items I still have listed through acquisition are from ancient leagues that I can't be bothered to relist.

However acquisition is still invaluable as a personal search tool. I often play in standard and I have a ton of stuff. There is no way I could find anything without acquisition (or looty). Even in leagues it's easier to just look for things there. So I believe this is the direction that you should take the app. Make it a better search tool. I don't expect it to rival the search engine but there's definite room for improvement there if you have time.Pseudo-mods are a great starting point.

On Tue, Jul 23, 2024 at 2:50 AM GERWARIC @.***> wrote:

That's good feedback. Since acquisition isn't popular these days, the only feedback I get about what features people use or care about comes from the ones willing to use GitHub to post issues.

Part of me wants to rebuild a new acquisition from scratch given changes GGG has made, and changes in technology over the last decade. The current guts are pretty complicated and tricky to understand. However, a more practical priority is to improve the mod searching, since it doesn't distinguish between implicit, explicit, and other modifier types the way that the trade site does. It also doesn't compute pseudo-mods at all. (It looks like it might have worked at some point in the past, but right now it's not).

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2243997720, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZO5LDWNU7RTAWUW2PDZNWLCVAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBTHE4TONZSGA . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

That's great to hear. Don't tell anyone just yet, but my long-term vision is to have three types of searches:

  1. Equipable items and jewels
  2. Stackable items, aka currency
  3. Unstackables, such as maps and logbooks

However, it's probably more immediately useful to get pseudo mods working. There's code that suggests they worked some time in the past, but I haven't looked into when that stopped working. Heck, it's even possible some of the early changes I made to acquisition did it, because I was learning c++ as well as acquisition's code base and GGG's API and data structures.

Coming back to the topic of this thread...

If you don't have any issues with the latest alpha, I'd like to release v0.11.1 in the next week or two. (Caveat: https://github.com/gerwaric/acquisition/issues/50 may become a blocking issue).

aiolos01 commented 4 months ago

No issues so far. I'll let you know if I find any.

On Thu, Jul 25, 2024 at 7:48 PM GERWARIC @.***> wrote:

That's great to hear. Don't tell anyone just yet, but my long-term vision is to have three types of searches:

  1. Equipable items and jewels
  2. Stackable items, aka currency
  3. Unstackables, such as maps and logbooks

However, it's probably more immediately useful to get pseudo mods working. There's code that suggests they worked some time in the past, but I haven't looked into when that stopped working. Heck, it's even possible some of the early changes I made to acquisition did it, because I was learning c++ as well as acquisition's code base and GGG's API and data structures.

Coming back to the topic of this thread...

If you don't have any issues with the latest alpha, I'd like to release v0.11.1 in the next week or two. (Caveat: #50 https://github.com/gerwaric/acquisition/issues/50 may become a blocking issue).

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2250965158, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZI6H4ULF7NCEDPIP5DZOET7RAVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJQHE3DKMJVHA . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 4 months ago

A few hours ago another user had their API access blocked at the network level due to a previously unknown bug in acquisition (https://github.com/gerwaric/acquisition/issues/50). Details along with an update here:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.12

I strongly recommend installing this update to avoid the risk of losing access to third-party tools until GGG has time to respond to any support requests you'd have to send.

aiolos01 commented 4 months ago

Yeah, I saw that. Damn that 0.11 version is cursed, I suggest skipping it altogether.

On Fri, Jul 26, 2024 at 7:32 AM GERWARIC @.***> wrote:

A few hours ago another user had their API access blocked at the network level due to a previously unknown bug in acquisition (#50 https://github.com/gerwaric/acquisition/issues/50). Details along with an update here:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1-alpha.12

I strongly recommend installing this update to avoid losing access to third-party tools until GGG has time to repsond to any support requests you'd have to send.

— Reply to this email directly, view it on GitHub https://github.com/gerwaric/acquisition/issues/39#issuecomment-2251955511, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSVXZLDHHOTA6M6Z5D7ZFLZOHGL7AVCNFSM6AAAAABJYZB6CWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJRHE2TKNJRGE . You are receiving this because you were mentioned.Message ID: @.***>

gerwaric commented 3 months ago

I have an idea of what happened now. If this build fixes it I'll write it up in more detail:

https://github.com/gerwaric/acquisition/releases/tag/v0.11.1.1-alpha.1

I'm also closing this issue for now, since I think the issues causing the crashes you were reporting have been fixed:

If these issues come back, please reopen this issue.