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

RPG Analyzer #48

Merged
merged 4 commits into from Jul 1, 2017

Conversation

Projects
None yet
2 participants
@gcabbage
Contributor

gcabbage commented Jun 17, 2017

This is a fix for Thermo shell nanofac causes UI issues affecting analyzers.

When the nanofac OnUpdate event is called the dsRPGAnalyzeItem item list gets reset (see also 60413) and the gItem variable is changed to the nanofac causing various glitches as the dockscreen tries to use the nanofac to analyze items.

This PR saves the analyzer item as screen data which solves most of the UI problems (apart from reseting to the start of the list). Switching to a customItemPicker fixes the list reset

@gmoromisato

This comment has been minimized.

Show comment
Hide comment
@gmoromisato

gmoromisato Jun 27, 2017

Member

I'd like to fix this in a different way. In particular, switching to customItemPicker seems wrong to me, since we'd have to switch all the ship dock screens to that. Instead, I'd like to fix the engine to preserve the list position.

Switching the item to screen data is a good change, since it avoids using global variables (always good) but I also want to fix the engine so that gItem does not get overwritten while you're in a dock screen (there is code to preserve it, but it fails in this particular case).

Bottom line, I'd like to take the "item as screen data" commit and the "analyzer destroys item" commit, but not the customItemPicker change.

I'll work on the engine changes this week.

Member

gmoromisato commented Jun 27, 2017

I'd like to fix this in a different way. In particular, switching to customItemPicker seems wrong to me, since we'd have to switch all the ship dock screens to that. Instead, I'd like to fix the engine to preserve the list position.

Switching the item to screen data is a good change, since it avoids using global variables (always good) but I also want to fix the engine so that gItem does not get overwritten while you're in a dock screen (there is code to preserve it, but it fails in this particular case).

Bottom line, I'd like to take the "item as screen data" commit and the "analyzer destroys item" commit, but not the customItemPicker change.

I'll work on the engine changes this week.

@gcabbage

This comment has been minimized.

Show comment
Hide comment
@gcabbage

gcabbage Jun 27, 2017

Contributor

I thought you'd prefer to fix it in the engine, but included the TLisp hack incase it was too far down the priority list. I'll rework the patch as requested (if I recall most of the "analyzer destroys item" commit is only needed due to the use of customItemPicker, but there is some tidying up that's worth saving)

BTW - the list reset issue only affects the case where the list comes from gSource e.g.

  • the sell items list (gSource=station player is docked at, but listing items from player) behaves correctly
  • rpganalyzer and jettision screens (gSouce = player, listing items from player) is affected by reset
Contributor

gcabbage commented Jun 27, 2017

I thought you'd prefer to fix it in the engine, but included the TLisp hack incase it was too far down the priority list. I'll rework the patch as requested (if I recall most of the "analyzer destroys item" commit is only needed due to the use of customItemPicker, but there is some tidying up that's worth saving)

BTW - the list reset issue only affects the case where the list comes from gSource e.g.

  • the sell items list (gSource=station player is docked at, but listing items from player) behaves correctly
  • rpganalyzer and jettision screens (gSouce = player, listing items from player) is affected by reset

@gmoromisato gmoromisato merged commit b5e1b9e into kronosaur:master Jul 1, 2017

@gmoromisato

This comment has been minimized.

Show comment
Hide comment
@gmoromisato

gmoromisato Jul 1, 2017

Member

Thank you!

Member

gmoromisato commented Jul 1, 2017

Thank you!

@gcabbage gcabbage deleted the gcabbage:rpganalyzer branch Sep 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment