Adding Image Preview to CampaignSelector#939
Adding Image Preview to CampaignSelector#939BlackgamerzVN wants to merge 17 commits intoCnCNet:developfrom
Conversation
|
Nightly build for this pull request:
|
DXMainClient/DXMainClient.csproj
Outdated
| <ItemGroup> | ||
| <ProjectReference Include="..\ClientGUI\ClientGUI.csproj" /> | ||
| <ProjectReference Include="..\ClientUpdater\ClientUpdater.csproj" /> | ||
| <ProjectReference Include="..\Rampastring.XNAUI\Rampastring.Tools\Rampastring.Tools.csproj" /> |
There was a problem hiding this comment.
Is this modification intended, necessary, and related with the PR topic?
| pnlMissionPreview = new XNAPanel(WindowManager); | ||
| pnlMissionPreview.Name = "pnlMissionPreview"; | ||
| pnlMissionPreview.X = 500; | ||
| pnlMissionPreview.Y = 60; | ||
| pnlMissionPreview.Width = 350; | ||
| pnlMissionPreview.Height = 220; | ||
|
|
||
| // Use built-in background drawing | ||
| pnlMissionPreview.PanelBackgroundDrawMode = | ||
| PanelBackgroundImageDrawMode.STRETCHED; | ||
|
|
||
|
|
||
|
|
||
| AddChild(pnlMissionPreview); |
There was a problem hiding this comment.
Provide an example that this control is both optional and can be configured via ini. Update the document to contain the ini usage.
There was a problem hiding this comment.
Please don't hit the resolve conversion button and leave it to the reviewer. This issue is not resolved because it seems this new panel is not optional -- instead of disabling it (you should) when such a panel does not exist, the panel is created
DXMainClient/Domain/Mission.cs
Outdated
| @@ -14,6 +15,8 @@ | |||
|
|
|||
| using Rampastring.Tools; | |||
|
|
|||
| using static System.Collections.Specialized.BitVector32; | |||
There was a problem hiding this comment.
Is this modification intended, necessary, and related with the PR topic?
Cleaned up excess codes and finished the feature.
Made the panel optional for use (for real). Also enabled it to be displayed when the mission is not enabled.
| pnlMissionPreview.BackgroundTexture.Dispose(); | ||
|
|
||
| pnlMissionPreview.BackgroundTexture = previewTexture; | ||
| previewNeedsPreplace = true; |
There was a problem hiding this comment.
This line will not work since previewNeedsPreplace is a local variable (so the true value is not saved when leaving this method). You should try defining private bool pnlMissionPreviewTextureNeedsDispose = false as a class member
|
|
||
| string relativePath = Path.Combine("Resources", mission.PreviewImage); | ||
|
|
||
| if (File.Exists(relativePath)) |
There was a problem hiding this comment.
Is this determination necessary? IIRC AssetLoader.LoadTextureUncached will return pink if relativePath does not exist (please check again)
| private void LoadPreview() | ||
| { | ||
| // find existing child first (the GUICreator/layout might already create it) | ||
| XNAPanel existingPanel = FindChild<XNAPanel>("pnlMissionPreview", true); |
There was a problem hiding this comment.
pnlMissionPreview = FindChild<XNAPanel>("pnlMissionPreview", optional: true);
if (pnlMissionPreview != null)
{
...|
Is there still any need to make pnlMissionPreview togglable? Because apparently everything in CampaignSelector.cs uses hardcoded value now. |
how about this. you define a file name for the default mission preview image, and if this image does not exist, the mission preview is null and the size of tbMissionDescription etc. remains the old; if the image exists, define the mission preview and the new size of tbMissionDescription etc. to make some rooms |
|
Lemme see if that can be done. For now I'm going to unhardcode controls first then put them in seperate PR before continuing on this. |
|
Actually fuck it. Imma be done with this. |
There isn't really a way to disable it per se. However it is now set to not be Visible by default (Which technically works).
Now Mission preview image are put into "Mission Previews" folder and defined in Battle.ini, whilst default preview image is also needed to be put into "Mission Previews" with an png named "Default Preview.png"
Been thinking about it. I don't think it really pan well for someone who has custom Campaign Preview setup. For now I can make it Visible=false by default. Will update description later. |
Replaced previewNeedsPreplace with pnlMissionPreviewTextureNeedsDispose and making pnlMissionPreviewTextureNeedsDispose a class member
|
I think that's pretty much it |
Can you detail why the approach is not well? |
| public override void Initialize() | ||
| { | ||
| // Read customizable window size from the campaign settings INI (fall back to defaults) | ||
| var settingsIni = new IniFile(SafePath.CombineFilePath(ProgramConstants.GamePath, SETTINGS_PATH)); |
There was a problem hiding this comment.
This is not the way we read a configuration.
Also, is it really infeasible to optionally enable the preview and resize the mission description panel? No one should expect an overlap between the preview and the mission description.
There was a problem hiding this comment.
Assuming image preview panel is to have 1/3 of the mission description panel's height, resize said description panel and place it at the bottom. would need to be properly scaled to 1:1 like with Skirmish and Multiplayer Lobby for starter, othersize it won't look well being stretched. And it doesn't take into account where mission description text may go pass that description panel.
Though I can place the mission preview at the bottom of campaign list then resize the campaign list instead to accomodate the mission preview like DTA. It's simpler to be done that way.
There was a problem hiding this comment.
would need to be properly scaled to 1:1 like with Skirmish and Multiplayer Lobby for starter, othersize it won't look well being stretched
Yes. What's the issue?
And it doesn't take into account where mission description text may go pass that description panel.
If the preview panel is enabled and the mission list is not resized, where should the panel be placed? and will the description text go pass the panel? I think yes the text will go pass the panel and thus it is also unexpected
There was a problem hiding this comment.
btw in case there is some misunderstanding -- we are talking about the default config right? the whole components can be configured via ini
| ClientRectangle = new Rectangle( | ||
| 0, | ||
| 0, | ||
| DEFAULT_WIDTH, | ||
| DEFAULT_HEIGHT); | ||
| BorderColor = UISettings.ActiveSettings.PanelBorderColor; |
There was a problem hiding this comment.
Do not make format-only changes unless you have significantly modified this file -- which is not
There was a problem hiding this comment.
My bad, I'll clean it up now.
You can now add image preview for individual missions in Campaign Selection.
Example in
CampaignSelector.ini:'[pnlMissionPreview]' is not Visible by default. Set Visible=true to '[pnlMissionPreview]' to enable it:
To insert the image preview, create a new folder named "Mission Previews" inside "Resources" and put your mission preview images there.
Any image named "Default Preview.png" in "Mission Preview" folder will be automatically taken as default for missions without image preview
To define which Preview each mission will use, in
Battle.ini: