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

AIM messages usually aren't printed to sidebar until it's closed #63368

Closed
Kamayana opened this issue Jan 31, 2023 · 7 comments · Fixed by #63575
Closed

AIM messages usually aren't printed to sidebar until it's closed #63368

Kamayana opened this issue Jan 31, 2023 · 7 comments · Fixed by #63575
Labels
(S1 - Need confirmation) Report waiting on confirmation of reproducibility

Comments

@Kamayana
Copy link
Contributor

Describe the bug

When moving individual items around using the AIM, messages are printed in the message box at the top of the AIM but not printed in the sidebar. When the AIM is closed, the sidebar is suddenly filled with all those messages at once. This does not occur when using Move All or when moving to All Surrounding Squares. It also doesn't occur if there's a nearby interruption, like an enemy, which thankfully makes this bug pretty minor.

This looks to be an unintended consequence of #62400 - it doesn't show up in the release immediately before that PR was merged.

Steps to reproduce

  1. Start default scenario.
  2. Go next to some items and open the AIM. Make one pane the items' tile and the other either another tile or the inventory.
  3. Move things around. Note that the AIM message box updates but the sidebar does not.
  4. Close the AIM. The sidebar should suddenly update with all of the messages.
  5. Open the AIM again and press ',' to move all items from one pane to another. Both the sidebar and the AIM messages should update properly without needing to close the AIM.
  6. Press 'a' to make one pane All Locations. Move items from the other pane to a tile and it should update both places.
  7. Debug spawn a zombie near the shelter NPC and move individual items. The sidebar should occasionally update as the zombie and the NPC fight, without needing to close the AIM.

Expected behavior

I'd expect the sidebar to always update as things are moved.

Screenshots

No response

Versions and configuration

  • OS: Windows
    • OS Version: 10.0.19045.2486 (22H2)
  • Game Version: 59f5604 [64-bit]
  • Graphics Version: Tiles
  • Game Language: System language []
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    No Fungal Growth [no_fungal_growth],
    Bionic Professions [package_bionic_professions]
    ]

Additional context

No response

@Kamayana Kamayana added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Jan 31, 2023
@ChromePoptart
Copy link

I had noticed this, too. The messages not showing up at time of transfer makes it difficult to track what you have already transferred -- I can't think of any specific examples off of the top of my head of when this has mattered, but I know I have noticed this issue and have in the moment wished that it would be reverted to the old, correct behavior. So, thanks for the write-up, @Kamayana!

@andrei8l
Copy link
Contributor

I can't reproduce but it's probably related to screen size/overlap between the AIM and side panel. Can you try this change?

diff --git a/src/advanced_inv.cpp b/src/advanced_inv.cpp
index a8e0b7bd7c..37af774f30 100644
--- a/src/advanced_inv.cpp
+++ b/src/advanced_inv.cpp
@@ -1635,6 +1635,7 @@ void advanced_inventory::display()
 
         if( ui ) {
             ui->invalidate_ui();
+            g->invalidate_main_ui_adaptor();
             ui_manager::redraw_invalidated();
         }
 

@Kamayana
Copy link
Contributor Author

Kamayana commented Feb 1, 2023

I can't reproduce but it's probably related to screen size/overlap between the AIM and side panel. Can you try this change?

diff --git a/src/advanced_inv.cpp b/src/advanced_inv.cpp
index a8e0b7bd7c..37af774f30 100644
--- a/src/advanced_inv.cpp
+++ b/src/advanced_inv.cpp
@@ -1635,6 +1635,7 @@ void advanced_inventory::display()
 
         if( ui ) {
             ui->invalidate_ui();
+            g->invalidate_main_ui_adaptor();
             ui_manager::redraw_invalidated();
         }
 

That worked! I'll make a PR for it, thanks!

@cake-pie
Copy link
Contributor

@Kamayana PR in wrong repo

@Kamayana
Copy link
Contributor Author

@Kamayana PR in wrong repo

Is it? The PR message looks correct to me:

Kamayana wants to merge 1 commit into master from aim_keep_updating_sidebar

Am I missing something?

@cake-pie
Copy link
Contributor

You made the PR in your own fork (Kamayana/Cataclysm-DDA) and not the upstream here (CleverRaven/Cataclysm-DDA)

@Kamayana
Copy link
Contributor Author

You made the PR in your own fork (Kamayana/Cataclysm-DDA) and not the upstream here (CleverRaven/Cataclysm-DDA)

Oh, Whoops! Thanks for pointing that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(S1 - Need confirmation) Report waiting on confirmation of reproducibility
Projects
None yet
4 participants