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.

Sign up for GitHub

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

Jump to bottom

Added voxel support to the assetbrowser #17803

Merged
merged 2 commits into from
Dec 25, 2020
Merged

Added voxel support to the assetbrowser #17803

merged 2 commits into from
Dec 25, 2020

Conversation

Mailaender
Copy link
Member

Closes #11492.

@Mailaender Mailaender mentioned this pull request Mar 23, 2020
Closed
@tovl
Copy link
Contributor

tovl commented Aug 2, 2020

This needs a rebase now.

@Mailaender
Copy link
Member Author

Rebased.

@tovl
Copy link
Contributor

tovl commented Aug 2, 2020

Doesn't compile right now due to the WRot changes.

@Mailaender
Copy link
Member Author

Sorry, that was naive and lazy. Fixed.

@tovl
Copy link
Contributor

tovl commented Aug 2, 2020

You might want to add labels to the roll, pitch and yaw sliders. Otherwise looks good

@tovl
Copy link
Contributor

tovl commented Aug 2, 2020

hmm, I see that a camera angle is defined, but it seems to always set the camera straight above?

@Mailaender
Copy link
Member Author

Mailaender commented Aug 3, 2020 •
edited
Loading

Added the labels. Also added in the unused camera angle parameter.

tovl
tovl previously approved these changes Aug 8, 2020
View reviewed changes
Copy link
Contributor

@tovl tovl left a comment

Choose a reason for hiding this comment

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

LGTM

@abcdefg30 abcdefg30 requested a review from pchote August 9, 2020 11:22
@pchote
Copy link
Member

pchote commented Aug 14, 2020 •
edited
Loading

The default camera angle, behaviour of the roll/pitch/yaw sliders, and behaviour of the shadow all feel a bit bogus. Did a rebase break these?

@tovl
Copy link
Contributor

tovl commented Aug 14, 2020

Could have been regressed by #18418 then. I don't think anything else could have resulted in the voxels being shown upside down upon a rebase.

@Mailaender
Copy link
Member Author

Rebased and improved the default camera angle.

pchote
pchote reviewed Aug 15, 2020
View reviewed changes
OpenRA.Mods.Common/Widgets/Logic/AssetBrowserLogic.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/Logic/AssetBrowserLogic.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ModelWidget.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ModelWidget.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ModelWidget.cs Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ModelWidget.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ModelWidget.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Widgets/ModelWidget.cs Outdated Show resolved Hide resolved
mods/common/chrome/assetbrowser.yaml Outdated Show resolved Hide resolved
mods/ts/sequences/voxels.yaml Outdated Show resolved Hide resolved
@Mailaender Mailaender force-pushed the voxel-assetbrowser branch 2 times, most recently from 418d183 to 6ca3581 Compare September 27, 2020 11:36
@abcdefg30
Copy link
Member

This was silently updated.

@abcdefg30
Copy link
Member

Actually, some of the review comments are not addressed. @Mailaender what is the plan here?

@Mailaender
Copy link
Member Author

I came up with a solution for all of them now. It just took me some time.

@abcdefg30
Copy link
Member

This works really nicely ingame. 👍

@abcdefg30 abcdefg30 requested a review from pchote October 24, 2020 09:23
@pchote pchote merged commit fb20479 into OpenRA:bleed Dec 25, 2020
@pchote
Copy link
Member

pchote commented Dec 25, 2020

https://github.com/OpenRA/OpenRA/wiki/Changelog-(bleed)/_compare/902cef58a06f68e77a04ba214bab456b0c6cc890

@Mailaender Mailaender deleted the voxel-assetbrowser branch December 25, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abcdefg30 abcdefg30 abcdefg30 left review comments

@pchote pchote pchote approved these changes

@tovl tovl tovl left review comments

Assignees
No one assigned
Labels
PR: Needs +2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Asset Browser doesn't show any voxels
4 participants
@Mailaender @tovl @pchote @abcdefg30