Page MenuHomeWildfire Games

Add particle support to actor editor
ClosedPublic

Authored by shh on Thu, Mar 7, 5:47 PM.

Details

Reviewers
vladislavbelov
wraitii
Yves
Itms
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22117: Add particle support to the Actor Editor, fixes #1590.
Trac Tickets
#1590
Summary

Added a column for particles in Actor Editor.
The column was added where I found it most suitable, but of course it may be changed if requested.

Test Plan

When loading particle actor, the <particle> file attribute is shown in particle column.
When saving an actor with file selected in particle column, it is saved as the name attribute for <particle> tag.
Seems to work fine.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

shh created this revision.Thu, Mar 7, 5:47 PM
shh added a reviewer: Restricted Owners Package.Thu, Mar 7, 5:52 PM
shh added a project: Windows Developers.
Stan added a subscriber: Stan.Thu, Mar 7, 10:12 PM

The patch looks good to me. I'm no programmer though so I can't give you a definitive review. I guess it's no question to add an actual editor for particles so patch is complete. Only missing to update the file license headers date.

However for future patches or iterations of this one please add context to your patch.

To do so if you are using SVN in the command line at the root of the repository type

svn diff -x -U5000 > {PatchName}.diff then upload that.

For git

git diff -U5000 > {PatchName}.diff

If you are on windows you can also directly put it into your clipboard.

Replace > {PatchName}.diff by | clip.exe

If you want to improve this tool a good way would be to figure out what makes it crash so badly on windows :)

asterix added a subscriber: asterix.

Thank you for your first contribution. Looks simple enough. I added respective guys that are knowledgeable in this area, of course, they can resign from this revision proposal. Please also keep in mind the advice that Stan gave you. Update dates please in those files.

shh updated this revision to Diff 7527.Fri, Mar 8, 2:40 PM

Done as advised by Stan.

Vulcan added a subscriber: Vulcan.Fri, Mar 8, 2:47 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1084/display/redirect

vladislavbelov added inline comments.Fri, Mar 8, 3:37 PM
source/tools/atlas/AtlasUI/ActorEditor/ActorEditorListCtrl.cpp
53 ↗(On Diff #7527)

Does it work correctly for all supported versions of wxWidget?

Stan added inline comments.Fri, Mar 8, 3:52 PM
source/tools/atlas/AtlasUI/ActorEditor/ActorEditorListCtrl.cpp
53 ↗(On Diff #7527)

Well given that's the same line than all the others it should.

Itms accepted this revision.Sat, Mar 16, 10:10 PM
Itms added a subscriber: Itms.

Works for me, thanks for this patch!

The placement of the new column looks good and sensible to me.

This revision is now accepted and ready to land.Sat, Mar 16, 10:10 PM
This revision was automatically updated to reflect the committed changes.
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Sat, Mar 16, 10:16 PM