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

Added dungeon rooms finisher. #1345

Merged
merged 9 commits into from Aug 27, 2014

Conversation

Projects
None yet
3 participants
@madmaxoft
Member

madmaxoft commented Aug 26, 2014

This implements #1260.

The DungeonRooms finisher adds dungeon rooms to the generated worlds. The rooms' placement is based on a GridStructGen, the rooms will contain 2 chests each.

@NiLSPACE

This comment has been minimized.

Show comment
Hide comment
@NiLSPACE

NiLSPACE Aug 26, 2014

Member

Vanilla doesn't check if the chests would form a double chest, so do we want to do the same?

Member

NiLSPACE commented Aug 26, 2014

Vanilla doesn't check if the chests would form a double chest, so do we want to do the same?

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 26, 2014

Member

I believe it does, because there are rendering errors if they are not checked:
bad_dblchest
(the second chest is in the highlighted block, but because the primary chest's meta is wrong, the renderer draws the doublechest to the wrong side)

Member

madmaxoft commented Aug 26, 2014

I believe it does, because there are rendering errors if they are not checked:
bad_dblchest
(the second chest is in the highlighted block, but because the primary chest's meta is wrong, the renderer draws the doublechest to the wrong side)

@NiLSPACE

This comment has been minimized.

Show comment
Hide comment
@NiLSPACE

NiLSPACE Aug 26, 2014

Member

Double chests are noted in the trivia from the wiki.
EDIT:
Maybe Mojang does some meta magic to make them look good?

Member

NiLSPACE commented Aug 26, 2014

Double chests are noted in the trivia from the wiki.
EDIT:
Maybe Mojang does some meta magic to make them look good?

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 26, 2014

Member

I think I'll use the path-of-least-resistance for the dblchest - I will make it so that no single dungeon generates its chest next to each other. This will fix the vast majority of the bad cases, the only remaining problems will be with overlapping dungeons; I will ignore them for the time being, because a proper solution might be impossible to implement (imagine someone setting the generator to generate a dungeon for each column, there would be so many chests that it might be impossible to place them properly)

Member

madmaxoft commented Aug 26, 2014

I think I'll use the path-of-least-resistance for the dblchest - I will make it so that no single dungeon generates its chest next to each other. This will fix the vast majority of the bad cases, the only remaining problems will be with overlapping dungeons; I will ignore them for the time being, because a proper solution might be impossible to implement (imagine someone setting the generator to generate a dungeon for each column, there would be so many chests that it might be impossible to place them properly)

@tigerw

This comment has been minimized.

Show comment
Hide comment
@tigerw

tigerw Aug 26, 2014

Member

"...profiling for x64..."
"...dungeons..."

Hm.

Member

tigerw commented Aug 26, 2014

"...profiling for x64..."
"...dungeons..."

Hm.

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 26, 2014

Member

I know I know, I'm turning into a tigerw :P

Member

madmaxoft commented Aug 26, 2014

I know I know, I'm turning into a tigerw :P

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 27, 2014

Member

I think this is now feature-complete. Comments?

Member

madmaxoft commented Aug 27, 2014

I think this is now feature-complete. Comments?

@NiLSPACE

This comment has been minimized.

Show comment
Hide comment
@NiLSPACE

NiLSPACE Aug 27, 2014

Member

Yea it looks nice. I'd say that we can merge.

Member

NiLSPACE commented Aug 27, 2014

Yea it looks nice. I'd say that we can merge.

madmaxoft added a commit that referenced this pull request Aug 27, 2014

@madmaxoft madmaxoft merged commit 4a907c9 into master Aug 27, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@madmaxoft madmaxoft deleted the DungeonRoomsFinisher branch Aug 27, 2014

@NiLSPACE

This comment has been minimized.

Show comment
Hide comment
@NiLSPACE

NiLSPACE Aug 27, 2014

Member

Did you forget one side?: Pic

Member

NiLSPACE commented Aug 27, 2014

Did you forget one side?: Pic

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 27, 2014

Member

No, all sides should be accounted for. Can you give me the coords and world.ini for that screenshot?

Member

madmaxoft commented Aug 27, 2014

No, all sides should be accounted for. Can you give me the coords and world.ini for that screenshot?

@NiLSPACE

This comment has been minimized.

Show comment
Hide comment
@NiLSPACE

NiLSPACE Aug 27, 2014

Member

I don't think that I can find the exact spot because I already moved on, but I'll search for another occurrence.

Member

NiLSPACE commented Aug 27, 2014

I don't think that I can find the exact spot because I already moved on, but I'll search for another occurrence.

@madmaxoft

This comment has been minimized.

Show comment
Hide comment
@madmaxoft

madmaxoft Aug 27, 2014

Member

Found it. An off-by-one error in the fast-bail-out checking.

Member

madmaxoft commented Aug 27, 2014

Found it. An off-by-one error in the fast-bail-out checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment