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

fix: forbid cows from origin and house spaces #1625

Merged
merged 27 commits into from
May 31, 2024
Merged

Conversation

evemartin
Copy link
Contributor

@evemartin evemartin commented May 28, 2024

This change is Reviewable

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @evemartin)


game/end_to_end_tests/test_level_editor.py line 97 at r1 (raw file):

        allowed_space = self.selenium.find_elements(By.CSS_SELECTOR, "rect[fill='#87e34d']")
        assert len(allowed_space) == 0
        

Remove extra whitespaces


game/static/game/js/level_editor.js line 1697 at r1 (raw file):

        function isValidPlacement(controlledCoord){
            var controlledNode = ocargo.Node.findNodeByCoordinate(controlledCoord, nodes);
            if (!controlledNode || isOriginCoordinate(controlledCoord) || isHouseCoordinate(controlledCoord))

Check with Laura but it seems there is no really clear feedback if you try to put a cow on a start block or a house block. The cow stays there, the only visible difference is that it doesn't snap to the road. Not sure what the feedback should be though, maybe Laura would have a preference.

Copy link
Contributor Author

@evemartin evemartin left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @faucomte97)


game/end_to_end_tests/test_level_editor.py line 97 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove extra whitespaces

Fixed!


game/static/game/js/level_editor.js line 1697 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Check with Laura but it seems there is no really clear feedback if you try to put a cow on a start block or a house block. The cow stays there, the only visible difference is that it doesn't snap to the road. Not sure what the feedback should be though, maybe Laura would have a preference.

I've checked with Laura - since no other object on the map (like traffic lights for instance) has feedback besides red/green highlighting for invalid/valid spaces, she suggested leaving it this way for now and considering how to give more user feedback in the future!

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @evemartin)

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @evemartin)

@evemartin evemartin merged commit 0718f93 into master May 31, 2024
7 checks passed
@evemartin evemartin deleted the fix--cow-placement branch May 31, 2024 11:03
evemartin added a commit that referenced this pull request May 31, 2024
* fix: forbid cows from origin and house spaces

* debug test

* debug test

* debug test

* test out change to test

* test splitting the tests

* test the test

* test change to house test

* add log line for debugging

* debug test

* debug test

* debug test

* debug test

* remove test to see if others work

* debug test

* add back test code

* fix typo

* debug test

* debug test

* debug test

* debug test

* test move with offset

* test out slight adjustment

* tidy up

* address PR comments

* Merge branch 'master' into fix--cow-placement

* Merge branch 'master' into fix--cow-placement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants