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

Implemented "level terrain" feature, fix #28 #1017

Merged
merged 3 commits into from
Aug 21, 2022

Conversation

DanielMowitz
Copy link
Contributor

This is an overall naïve implementation, in the sense that it copies the height of the initial selected node and applies it to the whole area.

The main factors for code complexity stem from:

  • my assumption that we want to level an arbitrary area and not always a rectangular area, which prevents some checks
  • my personal requirements for levelling behavior on sloped nodes (see below)

The first point might be mitigated though by deciding on only having rectangular selection for levelling (see #1016).

Sloped node behavior

Slopes should lower the terrain when the selection is pulled from the lower side:

image
levels to
image

They should raise the terrain when the selection goes the other way:

image
levels to
image

Edged slopes should behave the same:

image
levels to
image

In all screenshots the rectangle was dragged up from the lowest edge (I couldn't more meaningful screenshots, sorry).

@lizzyd710 lizzyd710 added the enhancement New feature or request label Aug 19, 2022
@lizzyd710
Copy link
Collaborator

Oh wow, this is awesome, thank you! I don't know why sonarcloud is throwing a fit, but all the checks pass so that's the key part. I'll play around with it and give a review.

Copy link
Collaborator

@lizzyd710 lizzyd710 left a comment

Choose a reason for hiding this comment

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

Oh wow, this is awesome!!!!! Great job! Will approve once the other convos get resolved, but going through it in-game it worked like a charm!

@DanielMowitz
Copy link
Contributor Author

As an aside, the format-files.sh script changed 16 additional files that I haven't touched at all. Maybe the Shell script/clang handles formatting a bit differently to the windows/js one?

@DanielMowitz
Copy link
Contributor Author

… and with this commit it even compiles 👍

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

1.2% 1.2% Coverage
0.0% 0.0% Duplication

@lizzyd710
Copy link
Collaborator

As an aside, the format-files.sh script changed 16 additional files that I haven't touched at all. Maybe the Shell script/clang handles formatting a bit differently to the windows/js one?

Maybe. It's also possible that those files were formatted wrong. (As you saw in some other files you changed in this PR someone was doing weird formatting so that's entirely possible.)

Copy link
Collaborator

@lizzyd710 lizzyd710 left a comment

Choose a reason for hiding this comment

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

You're a superstar for this, especially since some of our contributors had to step back from the project. Much appreciated!!!

@lizzyd710 lizzyd710 merged commit cb327cd into CytopiaTeam:master Aug 21, 2022
@DanielMowitz DanielMowitz deleted the levelTerrain branch August 25, 2022 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Level terrain button doesn't work [Feature] Add "level terrain" feature
2 participants