Replies: 7 comments 39 replies
-
|
Interesting! I didn't know there was already had log parsing. I'm probably going to pass on this for now, I would personally rather work on making the price checking stuff better. I may come back to this later but probably not going to go core for now. |
Beta Was this translation helpful? Give feedback.
-
|
I have the feature working and for last few trade it seems to work fine. In terms of what we actually got here:
In terms what could be added:
What I didn't get in, due to technical limitations:
So @Kvan7, opinions? |
Beta Was this translation helpful? Give feedback.
-
|
Feel free to make a PR and I'll take a look at it. My current stance is still that stuff like Poe Lurker probably do similar operations, and since it is a dedicated program they have more dev time for specific functionality. I do have a couple concerns, I'll take a look at the PR and decide if they are actually relevant or not. My main concern is that I really don't have dev time to maintain another feature right now. This may get added but the PR may be open for a few days or a few months depending on how long it takes for me to get the price check to where I think I can call it good. I myself am personally fine with the chat hotkeys, I don't really get enough whispers to need a dedicated trade macro, so that is kinda where I am coming from when I was initially saying this may not go to core. |
Beta Was this translation helpful? Give feedback.
-
|
I admit it started as a 'I dont want to remember what hotkey I assigned to a message', but I got over that quite quickly. The main part for me now, is that I get a list of trade's I've missed while doing maps. I understand that it takes some time from you for reviews and testing, but in terms of fixes or improvements to this feature I do plan to stick around and do them. So I don't plan to leave you alone on this one ;) |
Beta Was this translation helpful? Give feedback.
-
|
PR: #386 |
Beta Was this translation helpful? Give feedback.
-
|
@Kvan7 did you have a moment to look into the PR? |
Beta Was this translation helpful? Give feedback.
-
|
I haven't looked at it yet, sorry. Work has been "not fun" last week so I didn't really do any coding this weekend. I'll try and look at it this week though |
Beta Was this translation helpful? Give feedback.
-
|
Small offtop in terms of what can we do with code. Response to #375 (reply in thread) Poe Lurker or Overwolf PoE Overlay 2 does exactly the same looking at their code. Looking at Lurker code, it seems the events stayed the same and they're guessing in what state the trade is currently in. An example trade looks like this, whole logs, including (but redacted) poe2 errors and global chat: My biggest issue at guessing is that I can not trust the "Trade accepted." - trading with a player and opening npc shop to dismantle uniques give the same line. |
Beta Was this translation helpful? Give feedback.
-
|
pretty disappointing. @tmakuch put in the work and offered help to implement and maintain, and @Kvan7 refused.. :( I don't want to run multiple programs, especially since poe2overlay (overwolf) tanks performance in some cases (I loose about 20 fps despite having powerful up-to-date hardware). I am not a big time trader and I do not need much polish, a simple trade viewer like shown in the video is all I wanted. @tmakuch is there any way to release this as a separate project that installs as an "addon" type of thing? thanks for your work! it is really appreciated. |
Beta Was this translation helpful? Give feedback.
-
|
@nightah latest commit check user agent and always show trade viewer outside of electron overlay, please check if this fixes browser for you |
Beta Was this translation helpful? Give feedback.
-
|
I have also added some changes alongside which allows the usage of the When I commit the change, it would be good @Kvan7 and @SnosMe if you guys can let me know if you're okay with where everything is and how I added a new event for this purpose. If committing back upstream outside of EE the location might need to change to the overlay window or somewhere more appropriate. |
Beta Was this translation helpful? Give feedback.
-
Given that you can't interact with any of the buttons to send messages etc I think that should still stay hidden, but removing an entry should stay so I would suggest the following: Index: renderer/src/web/client-log/WidgetTradeViewer.vue
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/renderer/src/web/client-log/WidgetTradeViewer.vue b/renderer/src/web/client-log/WidgetTradeViewer.vue
--- a/renderer/src/web/client-log/WidgetTradeViewer.vue (revision 8c2ec41e3df86d5e63619234b98ff7eb3b54b4fe)
+++ b/renderer/src/web/client-log/WidgetTradeViewer.vue (date 1741446302548)
@@ -48,7 +48,7 @@
v-for="(buyer, buyerIdx) in Object.keys(trade.buyers).filter(
(_, idx) => idx < 4,
)"
- v-if="!isElectron || !isMinimized"
+ v-if="!isMinimized"
class="flex flex-row gap-1 items-center mt-1 leading-4"
>
<divHaving said that, it looks like if something is removed from the browser, the overlay isn't updated, same thing in reverse too, you need to refresh the browser tab for it to force an update. |
Beta Was this translation helpful? Give feedback.
-
This is caused by the list of trades not being stored in the config but just an array created on the widget. You're not refreshing the list but empting it when you refresh. That was done by design - no need to clear the config if you exit ee2 or poe, so every time you start the game you start fresh.
Looks good, but is the ee2 usuable in any form inside a webbrowser? stash search won't work as well, not sure about keyboard shortcuts/trade search |
Beta Was this translation helpful? Give feedback.
-
Stash search works with keyboard shortcuts as long as POE is focused. For Trade search when it intercepts the hotkey and the browser window is open, it triggers inside the browser instead the overlay, so it certainly works. I should have clarified the overlay certainly works in Linux. However, in my environment if I want to benefit from the trade viewer I have to run the game in Windowed Mode. Both Fullscreen and Windowed Fullscreen will only show the widget when the overlay mode is activated so I won't be able to see what trades are active at a glance without bringing the overlay up. This might be completely related to my desktop environment/window manager. I'll do some more testing but I think it's likely just an X11 limitation with full screen and how layers work in that mode. My hope was that I could have the trade viewer up in the browser so I can see what trades messages have been sent through and be able to cancel/remove them from the browser but if I want to actually message/trade/interact I can go via the overlay. So some reactivity between the two could be ideal. Another thought was that given we have all the trades messages in the client file being able to bring back a history view of trades and trigger adding something back into the widget stack would be useful too. If you accidentally remove an item or want to speak to someone after a crash/restart or even testing this could be useful. I guess one of the things to consider in that scenario is the time since trade message. We likely will need to calculate it from the timestamp if we replayed it. Or I even would be open to the idea of replayed messages from history indicating they are as much if we wanted to avoid that. If you wanted to have a chat on Discord you can reach me at the same handle as GitHub. |
Beta Was this translation helpful? Give feedback.
-
|
@Kvan7 offtopic question - how do you style your code? Prettier or something default from the tool you're using? I don't usually use vue, so I do not have anything set up yet |
Beta Was this translation helpful? Give feedback.
-
|
yeah, just prettier and eslint. the eslint rules are kinda weird but it seems to work for now. In vscode i have the formatter set to "Prettier" with the eslint, prettier, and eslint+prettier extensions installed(there is also a vue one). I can add stuff to the vscode workspace config for recommended extensions since that may help out. For actually formatting, I really just have the default vscode formatter set to prettier (which is set to use the repos settings), and have format on save enabled. I think there is a way to run from cli as well but I have honestly never tried to use it. |
Beta Was this translation helpful? Give feedback.
-
|
If you're using some custom prettier settings then please share your prettierrc. I'll personally not benefit from vscode settings (different ide), but if you have something that is always a nice thing to share for other maintainers :) In general even if you're running default configs it's a good practice to have configs for them - it informs what you're running + most ide will pick those up automatically. |
Beta Was this translation helpful? Give feedback.
-
|
Ok, added a prettier script to the package.json of the renderer project 0f82b6c |
Beta Was this translation helpful? Give feedback.
-
|
Updated designs, with overlay hidden: |
Beta Was this translation helpful? Give feedback.
-
|
@Kvan7 I'm also happy to assist you and @tmakuch to get this feature over the line and add some other QoL elements too like features that the other trade apps have; sounds/notifications, would align well. As a Linux user, EE2 is the best option and in some cases the only option, MercuryTrade and SlimTrade both barely work with the latter requiring some additional effort (which resulted in a PR from me) to get it working. I was going to build something completely separate to emulate what Mercury and Slim do, having it all-in-one within EE2 would be a no-brainer though especially with @tmakuch already doing a significant portion of the heavy lifting. |
Beta Was this translation helpful? Give feedback.
-
|
@tmakuch you broke the tradeviewer with tmakuch@1b6e761. Looks like you're probably only relying on the configuration being resolved automatically and not catering for the scenario where a user has specified the location of the file to watch. Below patch fixes it, feel free to ensure it continues to work for yourself too: Index: main/src/host-files/GameLogWatcher.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/src/host-files/GameLogWatcher.ts b/main/src/host-files/GameLogWatcher.ts
--- a/main/src/host-files/GameLogWatcher.ts (revision 590f396ea6fc285f901eb05b868f95ca7eac3f45)
+++ b/main/src/host-files/GameLogWatcher.ts (date 1741407821779)
@@ -34,7 +34,6 @@
export class GameLogWatcher {
private _wantedPath: string | null = null;
- private _resolvedPath: string | null = null;
private _isEnabled: boolean = false;
get actualPath() {
return this._state?.path ?? null;
@@ -82,28 +81,27 @@
if (!logFile.length) {
const guessedPath = await guessFileLocation(POSSIBLE_PATH);
if (guessedPath === null) {
- this._resolvedPath = null;
return;
}
- this._resolvedPath = guessedPath;
+ this._wantedPath = guessedPath;
}
}
async tryStart() {
- if (!this._resolvedPath || !this._isEnabled) {
+ if (!this._wantedPath || !this._isEnabled) {
return;
}
try {
- const file = await fs.open(this._resolvedPath, "r");
+ const file = await fs.open(this._wantedPath, "r");
const stats = await file.stat();
watchFile(
- this._resolvedPath,
+ this._wantedPath,
{ interval: 450 },
this.handleFileChange.bind(this),
);
this._state = {
- path: this._resolvedPath,
+ path: this._wantedPath,
file,
offset: stats.size,
isReading: false, |
Beta Was this translation helpful? Give feedback.
-
|
Bug indeed for a case, when someone provides path themself. But this will trigger rewatch on every settings change if you do not have path provided, since ee2 will call setup with empty string, while _wantedPath was updated. @nightah can you please check latest commit fixes the issue? It feels like it simply lacking assumption that provided paths is the expected resolved file. For not-user-provided case it will be resolved by guessFileLocation anyway, for user-provided case it will be used. async setup(logFile: string) {
if (this._wantedPath === logFile) {
return;
}
this._wantedPath = logFile;
if (this._state) {
unwatchFile(this._state.path);
await this._state.file.close();
this._state = null;
}
+ this._resolvedPath = logFile;
if (!logFile.length) {
const guessedPath = await guessFileLocation(POSSIBLE_PATH);
if (guessedPath === null) {
this._resolvedPath = null;
return;
}
this._resolvedPath = guessedPath;
}
} |
Beta Was this translation helpful? Give feedback.
-
Confirmed this resolved it too. |
Beta Was this translation helpful? Give feedback.
-
|
I was also thinking about this comment of yours:
We could make the thank and trade buttons both conditionally appear after the player has been detected entering in our zone (which I believe does have a log message). |
Beta Was this translation helpful? Give feedback.
-
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Hey @Kvan7,
I started to play again with EE2 by trying to add a new feature, just to learn backend is already there.
I've started to add a widget that will show you trade whispers in case you would miss some. Something like:
And it turns out the client log file is already being read and each line is parsed nicely with "MAIN->CLIENT::game-log" event and https://github.com/Kvan7/Exiled-Exchange-2/blob/master/renderer/src/web/client-log/client-log.ts#L38
Are there any plans to utilize this code or can I have a go at this with proposed feature? As far as I can tell, client logs are nicely parsed but then left unattended.
Beta Was this translation helpful? Give feedback.
All reactions