-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Scenery boundbox refactor #21863
base: develop
Are you sure you want to change the base?
Scenery boundbox refactor #21863
Conversation
4f850ab
to
7c87c0d
Compare
7c87c0d
to
545477d
Compare
What is the point of the sprite offset part of this. You can already specify the offset of a sprite when specifying your individual sprites. This just leads to multiple places you can set the same value. |
Small scenery has an offset value of (1, 1, height), (3, 3, height), (15, 15, height) or (7, 7, height) depending on what flags are set - flags that control multiple behaviors. Large scenery has an offset value of (0, 0, height) always. Being able to manually set the offset value allows small and large scenery to draw sprites the same way. |
Yes we need to support the old flags but we shouldn't provide a new way to do this as you can already specify arbitrary offsets in the sprite. Large scenery does things correctly image offsets should always be 0,0 in paint functions, unfortunately small scenery did this wrong but we will have to keep supporting that. |
545477d
to
5710a3c
Compare
The 2D sprite coordinates are not redundant with regards to the 3D sprite offset on small scenery, because the 3D sprite offsets are tied to behavior flags. If a behavior flag changes, so does the 3D sprite offset. That means all the 2D sprite coordinates have to change to compensate. If a user mis-configures the object and sets the sprite coordinates based on that, all that work is undone when they configure the object correctly. That is bad user experience, whether the object has 4 sprites or 8 or 1024. The dilemma of accepting a mis-configured object versus the time to update all the 2D sprite coordinates is an unnecessary one that this PR solves. This truth table shows the normal combinations of flags, their sprite offsets, and features that the flags enable/disable:
Memorizing this should not be required to make small scenery, and changing the sprites' coordinates based on these flags should not be required either. |
0b8aa87
to
42be67f
Compare
3e6b969
to
3867482
Compare
3867482
to
8da2c36
Compare
I fixed the compiler linking issue. To others who have issues where the game will compile on all platforms except windows, be careful of what files are marked as |
67bd23e
to
0c94614
Compare
This PR adds user-definable bounding boxes to small and large scenery. It streamlines small and large scenery paint code by moving branches and calculations into import code instead of evaluating during paint. With user-definable bounding boxes, scenery can be created with bounding boxes that match the visuals of the scenery. This has the potential to reduce z-fighting, giving builders more freedom and expanding options.
The PR is divided into two commits which should be squash-merged. The first PR implements new loading and painting behavior, but keeps the existing paint behavior to verify the results are identical. The second PR removes the old behavior entirely, leaving only new behavior.
I have created a few test objects to test import behavior. Note that the
ipdeco
objects will trip the asserts of the first commit because of their custom bound boxes.bboxtest objects.zip