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

New grid related features #931

Closed
wants to merge 11 commits into from
Closed

Conversation

plrusek
Copy link
Contributor

@plrusek plrusek commented May 25, 2022

Description

  1. The grid now has been moved from being rendered in the background of the canvas (meaning it could be covered up by any room object easily) to the foreground of the canvas (meaning it will render on top of everything), thanks to the use of an opacity mask on the canvas.
  2. The automatically calculated grid height and width per room (based on the width and height of the most used tile in said room) can now be overriden by global properties that can be enabled/disabled and then set in the settings.
  3. The settings window itself had a slight reordering of it's contents (slighty tidyup, empty horizontal space was filled out)

plrusek added 7 commits April 22, 2022 21:24
… Tab

focus on object list in rooms when clicking on something
… the automatic grid calculation (from most used tile in a room).

Moved the grid from background to foreground (thanks to OpacityMask).
Adjusted the settings menu layout a bit.
Room thickness is 0.5 by default now.
@github-actions
Copy link

github-actions bot commented May 25, 2022

Add empty lines back in

Co-authored-by: VladiStep <vlad2001.21@mail.ru>
@@ -832,6 +832,7 @@ private void SelectObject(UndertaleObject obj)

resListView.IsExpanded = true;
resListView.BringIntoView();
resListView.Focus();
Copy link
Member

Choose a reason for hiding this comment

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

It should be after line 856 (resItem.IsSelected = true;).

@@ -1478,7 +1478,7 @@ private void TreeView_SelectedItemChanged(object sender, RoutedPropertyChangedEv

private void MainTree_MouseDoubleClick(object sender, MouseButtonEventArgs e)
{
OpenInTab(Highlighted);
OpenInTab(Highlighted, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is not mentioned on the PR description.
I personally do not like everything opening in a new tab.

@@ -403,11 +403,18 @@ public void SetupRoom(bool calculateGrid = true)
}

// If tiles exist at all, grab the most used tile size and use that as our grid size
// If a default starting grid is set in the settings, use that instead
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is slightly misleading, as in this code block the starting grid is not get from the settings as the grid isn't even recalculated in that case.

UndertaleModLib/Models/UndertaleRoom.cs Outdated Show resolved Hide resolved
UndertaleModTool/Editors/UndertaleRoomEditor.xaml.cs Outdated Show resolved Hide resolved
UndertaleModLib/Models/UndertaleRoom.cs Outdated Show resolved Hide resolved
@Miepee
Copy link
Contributor

Miepee commented May 25, 2022

Input boxes need some work:

  • I can add a lot of numbers here image
  • As mentioned in the comments from above, only setting height currently does not work as expected.
  • For a better user experience, it would be better if one couldn't input normal text in there
  • Default grid sizes should probably be 20x20 instead of 0x0
  • Setting negative grid sizes crashes the room editor
  • putting too high or too low numbers shows an error message. Would be better if a user couldn't input them in the first place, or if the numbers would be snapped to what makes sense (aka ]0;Int32.MaxInt])
  • same as above but with NaN and -NaN
  • -0 is apparently a valid grid size
  • Having thickness be enforceable via settings would also be nice
  • Exporting the room as a PNG exports the grid as well which is undesirable. EDIT: Nevermind, seems that it is actually desirable considering that the ExportAllRoomsToPNG script asks whether or not to draw the grid.

plrusek added 2 commits May 25, 2022 20:38
…grid changes to UndertaleRoomRenderer. Condensed some if statements. Reverted default grid thickness.
<DrawingBrush TileMode="Tile" ViewportUnits="Absolute">
<DrawingBrush.Viewport>
<MultiBinding Converter="{StaticResource GridConverter}">
<Binding Path="GridWidth" Mode="OneWay"/>
Copy link
Member

Choose a reason for hiding this comment

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

All the data bindings that you've changed/added should be OneTime.

@Miepee
Copy link
Contributor

Miepee commented Jun 20, 2022

Vlad brought up something in the server which I think is worth noting here.
Currently, with the revamped settings, columns are a bit hard to distinguish.
image

Something that could help with this, would be to move the checkboxes before the text, similar to how other tools do it. (using JBRider here as an example:)
image
The above path textboxes can be left as-is, but the textboxes for width/height should be then also rearranged to have the activation checkbox on the left, with the textbox on the right like here:
image

@Miepee
Copy link
Contributor

Miepee commented Jun 30, 2022

Thanks for the PR! Unfortunate, that you weren't able to get this one merged and that we needed to create our own PRs for it, but nevertheless your contributions are very welcomed :)

@Grossley
Copy link
Collaborator

No longer necessary.

@Grossley Grossley closed this Jun 30, 2022
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.

4 participants