feat: enable cow crossing condition, and horn action #1376

sebp999 · 2022-11-07T15:06:04Z

Description

Fixes #1334 #1335 #1843 #952 (partly)

Add

Cow crossing condition block
Horn action, which removes cows from adjacent blocks
Block ordering for display
A cow as scenery, which can be positioned on the road.

Remove

declare_event block and corresponding block type
puff up
puff down actions

The cows are now not random but set up, like traffic lights. Horns remove all cows, so puffing up and down to remove brown cows is also gone.

How Has This Been Tested?

Manually using several scenarios. Cow collisions are already tested. The next PR (will be added later) adds a simple level demonstrating cows and the corresponding runthrough test will test it.

Checklist:

[ x ] My change requires a change to the documentation.
I have updated the documentation accordingly.
[ x ] I have linked this PR to a ZenHub Issue.

This change is

faucomte97

Reviewed 7 of 16 files at r1, 10 of 27 files at r2, 16 of 16 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @sebp999)

game/migrations/0077_auto_20221106_1608_data.py line 1 at r4 (raw file):

# Generated by Django 3.2.15 on 2022-11-06 16:08

Can you rename this migration please to reflect what it does? (although I can see it does a few things so maybe something like update_blocks?

game/migrations/0077_auto_20221106_1608_data.py line 37 at r4 (raw file):

        Block = apps.get_model("game", "Block")
        Block.objects.filter(type="puff_up").delete()
        Block.objects.filter(type="declare_event").delete()

Just using get instead of filter should work here

Code quote:

        Block.objects.filter(type="puff_up").delete()
        Block.objects.filter(type="declare_event").delete()

game/migrations/0078_merge_20221127_1108.py line 1 at r4 (raw file):

# Generated by Django 3.2.16 on 2022-11-27 11:08

Do we need this merging of the migrations?

game/migrations/0079_alter_block_options.py line 1 at r4 (raw file):

# Generated by Django 3.2.16 on 2022-11-27 11:09

If I understand correctly this migration exists because you've edited the Block model again after adding the options.
Maybe it would be cleaner to delete migrations 79 and 76, and re-run makemigrations which would combine them anyway. And then keep 77 as you have it now. That way we end up with just 2 migrations overall

game/static/game/js/map.js line 50 at r4 (raw file):

            nextCoordinates.x = currentNode.coordinate.x + 1;
            if (nextCoordinates.x > 9) { // right-most extremity
                return null;

extra whitespaces here

game/static/game/js/map.js line 59 at r4 (raw file):

        }
    } else { // moving vertically
        nextCoordinates.x = currentNode.coordinate.x;

Extra whitespace here

game/static/game/js/model.js line 106 at r4 (raw file):

        if (cow) {
            return true;
        }

Extra whitespace

game/static/game/js/model.js line 439 at r4 (raw file):

        };
    });

Extra whitespaces

game/static/game/js/model.js line 581 at r4 (raw file):

    for (var i = 0; i < this.trafficLights.length; i++) {
        var light = this.trafficLights[i];

Extra whitespaces

game/static/game/js/model.js line 610 at r4 (raw file):

                if (typeof(state) === "string") {
                    state = [state];
                }

Extra whitespace

game/static/game/js/model.js line 613 at r4 (raw file):

                if (state.includes(cow.activeNodes[jsonCoordinate])) {
                    return cow;
                }

Extra whitespace

game/templates/game/level_editor.html line 341 at r4 (raw file):

                      <td><img id="cow" src="/static/game/image/Clarice.svg" width="80"></td>
                      <td>

Extra whitespaces

sebp999

Reviewable status: 27 of 36 files reviewed, 12 unresolved discussions (waiting on @faucomte97)

game/static/game/js/map.js line 50 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

extra whitespaces here

Done.

game/static/game/js/map.js line 59 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespace here

Done.

game/static/game/js/model.js line 106 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespace

Done.

game/static/game/js/model.js line 439 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespaces

Done.

game/static/game/js/model.js line 581 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespaces

Done.

game/static/game/js/model.js line 610 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespace

Done.

game/static/game/js/model.js line 613 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespace

Done.

game/templates/game/level_editor.html line 341 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra whitespaces

Done.

game/migrations/0078_merge_20221127_1108.py line 1 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Do we need this merging of the migrations?

Removed

game/migrations/0077_auto_20221106_1608_data.py line 1 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Can you rename this migration please to reflect what it does? (although I can see it does a few things so maybe something like update_blocks?

Done.

game/migrations/0077_auto_20221106_1608_data.py line 37 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Just using get instead of filter should work here

I just prefer filter because type isn't a unique constraint and if there were 2 then get would break

game/migrations/0079_alter_block_options.py line 1 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

If I understand correctly this migration exists because you've edited the Block model again after adding the options.
Maybe it would be cleaner to delete migrations 79 and 76, and re-run makemigrations which would combine them anyway. And then keep 77 as you have it now. That way we end up with just 2 migrations overall

Done.

faucomte97

Reviewed 6 of 9 files at r5, all commit messages.
Reviewable status: 33 of 36 files reviewed, 3 unresolved discussions (waiting on @sebp999)

faucomte97

Reviewed 3 of 9 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sebp999)

game/migrations/0077_auto_20221106_1608_data.py line 37 at r4 (raw file):

Previously, sebp999 wrote…

I just prefer filter because type isn't a unique constraint and if there were 2 then get would break

ok 👍

game/migrations/0079_alter_block_options.py line 1 at r4 (raw file):

Previously, sebp999 wrote…

Done.

Looks good, although I think you're gonna have to just change the numbering now because we've since merged a new migration that we've labelled as 0077, so this one will now need to be 0078 and the next one 0079

sebp999

Reviewable status: 30 of 36 files reviewed, 1 unresolved discussion (waiting on @faucomte97)

game/migrations/0079_alter_block_options.py line 1 at r4 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Looks good, although I think you're gonna have to just change the numbering now because we've since merged a new migration that we've labelled as 0077, so this one will now need to be 0078 and the next one 0079

Done.

faucomte97

Reviewed 7 of 7 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @sebp999)

faucomte97

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

codecov · 2023-01-05T10:34:14Z

Codecov Report

Merging #1376 (6011b33) into master (7e31dc8) will increase coverage by 0.01%.
The diff coverage is 89.18%.

@@            Coverage Diff             @@
##           master    #1376      +/-   ##
==========================================
+ Coverage   91.52%   91.54%   +0.01%     
==========================================
  Files         100      102       +2     
  Lines        5512     5536      +24     
==========================================
+ Hits         5045     5068      +23     
- Misses        467      468       +1

Impacted Files	Coverage Δ
game/views/level_editor.py	`72.46% <66.66%> (ø)`
game/app_settings.py	`81.81% <100.00%> (ø)`
game/migrations/0078_add_block_types.py	`100.00% <100.00%> (ø)`
...rations/0079_populate_block_type_add_cow_blocks.py	`100.00% <100.00%> (ø)`
game/models.py	`95.12% <100.00%> (+0.12%)`	⬆️
game/random_road.py	`91.00% <0.00%> (-0.36%)`	⬇️

faucomte97

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @sebp999)

Seb Palmer and others added 3 commits November 7, 2022 14:55

feat: enable cow crossing condition, and horn action

020ba7e

feat: remove event listeners. Tidy migrations. Merge upstream.

2a7f7dc

Merge branch 'master' into 1334-cow-condition-and-sound-horn

f0ff44c

sebp999 requested a review from faucomte97 November 30, 2022 06:42

fix: fix error in placing cow

05e370c

This was referenced Nov 30, 2022

Cow condition block and sound horn action block #1335

Closed

Clean up the rest of the cow stuff ocadotechnology/codeforlife-portal#1843

Closed

Enabling cows on the level creator #1336

Closed

Understand the steps to enable Cow levels and Night Mode #952

Closed

faucomte97 requested changes Nov 30, 2022

View reviewed changes

refactor: change migrations, formatting js

09d4dbd

sebp999 commented Dec 2, 2022

View reviewed changes

faucomte97 reviewed Dec 9, 2022

View reviewed changes

faucomte97 requested changes Dec 9, 2022

View reviewed changes

Seb Palmer added 2 commits December 26, 2022 15:37

merge

3a285c4

chore: renumber migrations to catch up with master

6cf560a

sebp999 commented Dec 26, 2022

View reviewed changes

Seb Palmer added 2 commits December 27, 2022 19:36

refactor: remove duplicate cow_crossing and sound_horn blocks

cdd6c91

fix: add python for sound_horn and fix pre-existing error on deliver

22823d2

faucomte97 approved these changes Jan 4, 2023

View reviewed changes

Seb Palmer added 2 commits January 4, 2023 17:56

chore: catch up with master

f32a600

fix: Pipfile.lock

6011b33

sebp999 marked this pull request as ready for review January 5, 2023 07:48

faucomte97 approved these changes Jan 5, 2023

View reviewed changes

faucomte97 merged commit 3e25995 into ocadotechnology:master Jan 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable cow crossing condition, and horn action #1376

feat: enable cow crossing condition, and horn action #1376

sebp999 commented Nov 7, 2022 •

edited

Loading

faucomte97 left a comment

sebp999 left a comment

faucomte97 left a comment

faucomte97 left a comment

sebp999 left a comment

faucomte97 left a comment

faucomte97 left a comment

codecov bot commented Jan 5, 2023

faucomte97 left a comment

feat: enable cow crossing condition, and horn action #1376

feat: enable cow crossing condition, and horn action #1376

Conversation

sebp999 commented Nov 7, 2022 • edited Loading

Description

How Has This Been Tested?

Checklist:

faucomte97 left a comment

Choose a reason for hiding this comment

sebp999 left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

sebp999 left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 5, 2023

Codecov Report

faucomte97 left a comment

Choose a reason for hiding this comment

sebp999 commented Nov 7, 2022 •

edited

Loading