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

Editor undo redo #16772

Merged
merged 2 commits into from Sep 27, 2019
Merged

Editor undo redo #16772

merged 2 commits into from Sep 27, 2019

Conversation

@teinarss
Copy link
Contributor

teinarss commented Jul 13, 2019

Added Undo and redo functionality to the map editor with a new tab showing the history.

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 9af0129 to de944c2 Jul 13, 2019
@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from de944c2 to 3e6d607 Jul 13, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Jul 13, 2019

Updated

@xan2622

This comment has been minimized.

Copy link
Contributor

xan2622 commented Jul 25, 2019

@teinarss : can you share a GIF (with sharex or else) or screenshot of this feature ?

@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Jul 25, 2019

@Dzierzan

This comment has been minimized.

Copy link

Dzierzan commented Aug 11, 2019

Wow, that is nice and deeply needed.

@wippie-openra

This comment has been minimized.

Copy link

wippie-openra commented Aug 27, 2019

Awesome!

Copy link
Member

RoosterDragon left a comment

Fantastic feature you've added here!

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 3e6d607 to 5263eeb Sep 15, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 15, 2019

Updated and added the suggestion for the item text. Think we need to polish these texts a bit tho.

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 5263eeb to 963e354 Sep 16, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 16, 2019

Updated and changed the text on the items according to what was discussed on irc.

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 963e354 to 3617c0c Sep 16, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 16, 2019

Updated

@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Sep 17, 2019

I think this is getting close now. Found a few bugs:

  • When editing an actor, the Delete and Cancel buttons overlap.
  • If I edit an actor but make no changes, hitting OK still results in an entry in the history. Would be nice if a no-op edit resulted in no history changes.
  • Add an actor, the open the edit dialog. In the history tab, go back before the actor was added. The actor is removed but the edit dialog remains visible.
  • When navigating backwards in history, an open edit dialog will keep up with changes to the actor, but not whilst navigating forwarded.

I don't mind how edit dialogs acts whilst you navigate the history (or if we just prevent that from happening at all), as long as it makes sense and you can't get into weird states.

@wippie-openra

This comment has been minimized.

Copy link

wippie-openra commented Sep 17, 2019

Changes to actors ( changing name or health) can be easily undone already. Perhaps its easier to not display these changes in the list

@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 17, 2019

Changes to actors ( changing name or health) can be easily undone already. Perhaps its easier to not display these changes in the list

How do you mean?

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 3617c0c to 71ebf5b Sep 20, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 20, 2019

Updated. Closing the edit actor dialog when navigate through history.

Copy link
Member

RoosterDragon left a comment

If I edit an actor but make no changes, hitting OK still results in an entry in the history. Would be nice if a no-op edit resulted in no history changes.

Would still be a nice to have, but I'm otherwise happy.

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 71ebf5b to 5f6c8e1 Sep 21, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 21, 2019

If I edit an actor but make no changes, hitting OK still results in an entry in the history. Would be nice if a no-op edit resulted in no history changes.

Would still be a nice to have, but I'm otherwise happy.

Updated. Made the OK button disabled until any changes has been made. Think its better UX wise than just not adding anything to the log.

Copy link
Member

abcdefg30 left a comment

This crashes right away for me in TS when opening the History tab after placing a few actors:

Exception of type `System.NullReferenceException`: Object reference not set to an instance of an object.
   at OpenRA.Mods.Common.Widgets.Logic.SimpleTooltipLogic.<>c__DisplayClass0_0.<.ctor>b__0()
   at OpenRA.Widgets.Widget.DrawOuter()
   at OpenRA.Widgets.Widget.DrawOuter()
   at OpenRA.Widgets.Widget.DrawOuter()
   at OpenRA.Game.RenderTick()
   at OpenRA.Game.Loop()
   at OpenRA.Game.Run()
   at OpenRA.Game.InitializeAndRun(String[] args)
   at OpenRA.Program.Main(String[] args)
@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 5f6c8e1 to 3e449f2 Sep 27, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor Author

teinarss commented Sep 27, 2019

updated

@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 3e449f2 to 8bda5a4 Sep 27, 2019
@teinarss teinarss force-pushed the teinarss:editor_undo_redo branch from 8bda5a4 to 5b6bafc Sep 27, 2019
Copy link
Member

abcdefg30 left a comment

Great work!

@abcdefg30 abcdefg30 merged commit 76034c1 into OpenRA:bleed Sep 27, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Sep 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.