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

Allow for longer names in location picker #2685

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

petchema
Copy link
Collaborator

Answer to request #2684

  • set limit to 50 characters instead of 29 when SDF fonts (that are narrower) are enabled;
  • make that limit more accessible to modding (did I get that part right? I'm not familiar with that aspect)

@petchema petchema added UI Change or issue related to the User Interface refinement labels Aug 12, 2024
@KABoissonneault
Copy link
Collaborator

I'm not sure public is the right accessibility yet. Usually, we would make this protected, so that a mod providing a window override can use a custom value.
Public could be right if there was a way for mods to register to an event that would let it change the value after Setup() (so that its value wins over the default) but before ShowLocationPicker (so it actually gets used). This way, mods could customize this behavior without overriding the window. I haven't looked if we have any such event

@KABoissonneault
Copy link
Collaborator

I'm not sure I like the explicit check for SDF vs non-SDF. Is that what we do for books and other auto-formatted text boxes? I know quest text is manually formatted to support non-SDF, but I think books are handled in code

@petchema
Copy link
Collaborator Author

I'm not sure public is the right accessibility yet. Usually, we would make this protected, so that a mod providing a window override can use a custom value. Public could be right if there was a way for mods to register to an event that would let it change the value after Setup() (so that its value wins over the default) but before ShowLocationPicker (so it actually gets used). This way, mods could customize this behavior without overriding the window. I haven't looked if we have any such event

Okay, if simplest/most convenient way is subclassing, making it a protected virtual property

@petchema
Copy link
Collaborator Author

I tried

diff --git a/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallListPickerWindow.cs b/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallListPickerWindow.cs
index 714a084dc..05b46792a 100644
--- a/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallListPickerWindow.cs
    b/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallListPickerWindow.cs
@@ -81,6  81,8 @@ namespace DaggerfallWorkshop.Game.UserInterfaceWindows
             // Create list box
             listBox.Position = new Vector2(26, 27);
             listBox.Size = new Vector2(138, 72);
             listBox.RectRestrictedRenderArea = new Rect(listBox.Position, listBox.Size);
             listBox.RestrictedRenderAreaCoordinateType = BaseScreenComponent.RestrictedRenderArea_CoordinateType.ParentCoordinates;
             listBox.OnUseSelectedItem  = ListBox_OnUseSelectedItem;
             pickerPanel.Components.Add(listBox);
 
diff --git a/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallTravelMapWindow.cs b/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallTravelMapWindow.cs
index 4e4cf5d79..77d61d045 100644
--- a/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallTravelMapWindow.cs
    b/Assets/Scripts/Game/UserInterfaceWindows/DaggerfallTravelMapWindow.cs
@@ -1565,7  1565,6 @@ namespace DaggerfallWorkshop.Game.UserInterfaceWindows
         {
             DaggerfallListPickerWindow locationPicker = new DaggerfallListPickerWindow(uiManager, this);
             locationPicker.OnItemPicked  = HandleLocationPickEvent;
-            locationPicker.ListBox.MaxCharacters = LocationPickerMaxCharacters;
 
             for (int i = 0; i < locations.Length; i  )
             {

but somehow it didn't work (text not clipped inside the ListBox. I'm not sure what I'm missing

@petchema
Copy link
Collaborator Author

Also, the same issue happens with the list of spells (spell merchant, spellbook)

@petchema
Copy link
Collaborator Author

but somehow it didn't work (text not clipped inside the ListBox. I'm not sure what I'm missing

My current hypothesis is that ShowLocationPicker is calling AddItem() of the ListBox before its parent DaggerfallListPickerWindow has been SetUp(), which is a problem because it's AddItem() that propagates the RestrictedArea of the ListBox to its elements

@petchema
Copy link
Collaborator Author

My analysis was correct, and simplest fix I could think of was to subclass DaggerfallListPickerWindow to delay calls to listBox.AddItem(s) to the end of Setup()
I end up fixing locations list, spell mechant/spell book lists, bookshelves list

@petchema
Copy link
Collaborator Author

sdf locations list after
nosdf locations list after

@petchema
Copy link
Collaborator Author

books list before
books list after

@petchema
Copy link
Collaborator Author

spells list before
spells list after

@petchema
Copy link
Collaborator Author

Could be factorized further if we decide DaggerfallListPickerWindow should always use restricted area, or would benefit from a constructor with a List to initialize its content; I just tried to keep changes as local as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refinement UI Change or issue related to the User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants