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

Feat/networth sessions #863

Closed
wants to merge 42 commits into from

Conversation

C3ntraX
Copy link

@C3ntraX C3ntraX commented Nov 25, 2022

Hello @viktorgullmark,

as discussed I will send you the pull request.

This pull request contains:

  • Net worth sessions with a bunch of refactoring and optimizations

Other fixes:
FIXED: Networth overlay does not shows the currency in divine/ex when opened
FIXED: Networth overlay does sometimes not update the states/is not in sync with the app
FIXED: Wrong Time format for minutes(currently as month formated after hour): HH:MM => HH:mm

Other features:
Added: Add critical stash tabs with tooltip info for profile selection

Added to the base UI:
image
image

TODOs:
I'll pass on the color selection to you. Its defined in the theme.
Stepper optimizations?

Copy link
Collaborator

@sbsrnt sbsrnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall alrighty PR but a lot of inconsistency between Net Worth Session naming and typos that may cause fixing headaches should bugs occur.

In the i18n strings I suggest to stick with Net Worth Session

ExilenceNextApp/public/i18n/en/common.json Outdated Show resolved Hide resolved
ExilenceNextApp/public/i18n/en/common.json Outdated Show resolved Hide resolved
"income_based_on_last_inactive_tooltip_short": "Since last inactivity",
"income_based_on_last_hour_tooltip_short": "Since last hour",
"critial_stashtyp_unique_tooltip": "Attention, a separate request is made for each item in the unique stashtab. This can significantly increase snapshot duration.",
"critial_stashtyp_map_tooltip": "Attention, spearate requests are made for all tiles in the map stash tab, which are not atlas maps. This can significantly increase snapshot duration.",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"critial_stashtyp_map_tooltip": "Attention, spearate requests are made for all tiles in the map stash tab, which are not atlas maps. This can significantly increase snapshot duration.",
"critical_stashtype_map_tooltip": "Separate requests are made for every map in the map stash tab. This can significantly increase snapshot duration.",

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's for all unique and special maps. Normal maps cannot be requested since the introduction of the map stash tab.

Maybe we should write "for every non standard map"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"critial_stashtyp_map_tooltip": "Attention, spearate requests are made for all tiles in the map stash tab, which are not atlas maps. This can significantly increase snapshot duration.",
"critical_stashtype_map_tooltip": "Separate requests are made for every non standard map in the map stash tab. This can significantly increase snapshot duration.",

ExilenceNextApp/src/components/toolbar/Toolbar.tsx Outdated Show resolved Hide resolved
Comment on lines +47 to +50
setTimeout(() => {
electronService.ipcRenderer.send('closed');
// TODO: If closed to early, the states are not saved to indexDB; Is there any hook/callback?
}, 1000);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may need to add clearTimeout somewhere to avoid edge-case memory leaks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shoudn't be cleared, because this is a callback for the app "close" event. If the timeout does not trigger the ipcRenderer event, then the app fails to exit.

ExilenceNextApp/src/store/uiStateStore.ts Outdated Show resolved Hide resolved
ExilenceNextApp/src/utils/snapshot.utils.ts Outdated Show resolved Hide resolved
() => [
{
type: 'MapStash',
message: t('common:label.critial_stashtyp_map_tooltip'),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
message: t('common:label.critial_stashtyp_map_tooltip'),
message: t('common:label.critical_stashtype_map_tooltip'),

C3ntraX and others added 4 commits December 7, 2022 20:10
Co-authored-by: Sebastian Krzyżanowski <[email protected]>
Co-authored-by: Sebastian Krzyżanowski <[email protected]>
Co-authored-by: Sebastian Krzyżanowski <[email protected]>
Co-authored-by: Sebastian Krzyżanowski <[email protected]>
@C3ntraX
Copy link
Author

C3ntraX commented Dec 7, 2022

@sbsrnt I accepted all of your suggestions. For the ones I didn't take over, this has a reason or still needs to be clarified.

@sbsrnt sbsrnt closed this Nov 6, 2023
@sbsrnt
Copy link
Collaborator

sbsrnt commented Nov 6, 2023

Appreciate the effort to make the tool better, unfortunately ultimately everything has been shut down 😿

@C3ntraX
Copy link
Author

C3ntraX commented Nov 6, 2023

Yeah, but its already merged into https://github.com/exilence-ce/exilence-ce :) I took the pr over. We will release this the next days. Anyways thanks for the code review.

@C3ntraX
Copy link
Author

C3ntraX commented Nov 6, 2023

Exilence will also be integrated into poestack-sage as plugin

https://github.com/PoeStack/poestack-sage

@C3ntraX
Copy link
Author

C3ntraX commented Nov 6, 2023

This is due all the missing items poe ninja are missing. That are many!

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.

2 participants