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

feat: prev next buttons #1383

Merged
merged 7 commits into from
Dec 2, 2022
Merged

feat: prev next buttons #1383

merged 7 commits into from
Dec 2, 2022

Conversation

KamilPawel
Copy link
Contributor

@KamilPawel KamilPawel commented Dec 1, 2022

Description

How Has This Been Tested?

Checklist:

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

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 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @KamilPawel)


game/migrations/0077_alter_level_next_level.py line 19 at r1 (raw file):

            Level_Object_Wrapper.filter(name=level.name).update(
                next_level=next_level_update
            )

can you add some code which is the reverse of this please, just like we have in migration 0075 for example

Code quote:

    def add_next_level_to_last_levels(apps, schema_editor):
        Level = apps.get_model("game", "Level")
        db_alias = schema_editor.connection.alias
        Level_Object_Wrapper = Level.objects.using(db_alias)
        levels = Level_Object_Wrapper.filter(next_level=None).exclude(name="109")
        for level in levels:
            next_level_update = Level_Object_Wrapper.get(
                name=(str(int(level.name) + 1))
            )
            Level_Object_Wrapper.filter(name=level.name).update(
                next_level=next_level_update
            )

game/static/game/css/game_screen.css line 291 at r1 (raw file):

  padding: 0px;
}

Extra line


game/static/game/js/drawing.js line 1057 at r1 (raw file):

  // buttons are passed as html string..
  // hence this terribleness
  if (Array.isArray(buttons)) {

Can we have more comments for this function please


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

    if (pythonSliderPosition < 30) {
      pythonSliderPosition = 30
      codeWindowHeight = 30

What is this for?

Code quote:

    if (pythonSliderPosition < 30) {
      pythonSliderPosition = 30
      codeWindowHeight = 30

game/views/level.py line 68 at r1 (raw file):

                prev_level = prev_level.prev_level.all()[0]
        except:
            print("Fegcrjhsvunicoa khudsFAUL")

Kamil come on 😛

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 3 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @KamilPawel)


game/end_to_end_tests/base_game_test.py line 82 at r3 (raw file):

        path = reverse("play_default_level", kwargs={"levelName": str(level_name)})
        self._go_to_path(path)
        if close_popup:

Do we need this in the end?

Copy link
Contributor Author

@KamilPawel KamilPawel 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: all files reviewed, 6 unresolved discussions (waiting on @faucomte97)


game/end_to_end_tests/base_game_test.py line 82 at r3 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Do we need this in the end?

Nope


game/migrations/0077_alter_level_next_level.py line 19 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

can you add some code which is the reverse of this please, just like we have in migration 0075 for example

Done.


game/static/game/css/game_screen.css line 291 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Extra line

Done.


game/static/game/js/drawing.js line 1057 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Can we have more comments for this function please

Done.


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

Previously, faucomte97 (Florian Aucomte) wrote…

What is this for?

The slider in python level, limiting it so you cannot go too high up


game/views/level.py line 68 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Kamil come on 😛

That looks like 2am code 😂

Copy link
Contributor Author

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 9 of 14 files reviewed, 6 unresolved discussions (waiting on @faucomte97)

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 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @KamilPawel)


game/end_to_end_tests/base_game_test.py line 82 at r3 (raw file):

Previously, KamilPawel wrote…

Nope

Then remove it from the arguments as well 😉


game/migrations/0077_alter_level_next_level.py line 19 at r1 (raw file):

Previously, KamilPawel wrote…

Done.

Ok but you need to call it 😝

Copy link
Contributor Author

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

:lgtm:

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


game/end_to_end_tests/base_game_test.py line 82 at r3 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Then remove it from the arguments as well 😉

Done.


game/migrations/0077_alter_level_next_level.py line 19 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Ok but you need to call it 😝

😂

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 2 of 2 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @KamilPawel)

@KamilPawel KamilPawel enabled auto-merge (squash) December 2, 2022 12:30
@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Merging #1383 (cc9af13) into master (d54a2e3) will decrease coverage by 0.15%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1383      +/-   ##
==========================================
- Coverage   91.72%   91.57%   -0.16%     
==========================================
  Files          99      100       +1     
  Lines        5477     5506      +29     
==========================================
+ Hits         5024     5042      +18     
- Misses        453      464      +11     
Impacted Files Coverage Δ
game/migrations/0077_alter_level_next_level.py 77.77% <77.77%> (ø)
game/views/level.py 72.51% <88.23%> (+1.36%) ⬆️
game/end_to_end_tests/base_game_test.py 94.00% <100.00%> (-1.31%) ⬇️
game/end_to_end_tests/test_play_through.py 100.00% <100.00%> (ø)
game/messages.py 97.72% <100.00%> (ø)
game/models.py 95.00% <100.00%> (ø)
game/end_to_end_tests/game_page.py 91.27% <0.00%> (-3.49%) ⬇️
game/random_road.py 91.36% <0.00%> (+0.35%) ⬆️

@KamilPawel KamilPawel merged commit 203e769 into master Dec 2, 2022
@faucomte97 faucomte97 deleted the prev_next_buttons branch August 9, 2023 17:06
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