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

Fixes #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixes #2

wants to merge 2 commits into from

Conversation

JuiP
Copy link
Member

@JuiP JuiP commented May 20, 2020

  • Add support for all display resolutions ( This activity was written for 1200x900 display resolution)

  • Update sugargame to v1.3

This activity was written for a laptop with 1200*900 display resolution.
Many coordinates were hard coded, as a result parts of the game were out of focus for other display resolutions
@@ -2,6 +2,10 @@
from pygame import gfxdraw
from .rangable import Rangable
import random
from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

Copy link

@quozl quozl left a comment

Choose a reason for hiding this comment

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

Good start, thanks. Don't be person with hammer who treats every problem as a nail. 😁

PyCut.py Outdated
@@ -2,10 +2,12 @@
import pygame
from game import PyCutGame as PyCut
import cProfile
import gi
from gi.repository import Gdk
Copy link

Choose a reason for hiding this comment

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

Not necessary to import Gdk into a Pygame program.

PyCut.py Outdated

def main():
pygame.init()
pygame.display.set_mode((1200, 900), pygame.RESIZABLE)
pygame.display.set_mode((Gdk.Screen.width(), Gdk.Screen.height()), pygame.RESIZABLE)
Copy link

Choose a reason for hiding this comment

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

Passing (0, 0) will use the full screen width, no need to use Gdk here.

Copy link
Member

Choose a reason for hiding this comment

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

Wow, That is a new info for me!!

self.width = 1200
self.height = 900
self.width = Gdk.Screen.width()
self.height = Gdk.Screen.height()
Copy link

Choose a reason for hiding this comment

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

Use either pygame.display.Info to get current_h and current_w, or use pygame.display.get_surface and then get_width and get_height.

@@ -1,6 +1,8 @@
import pygame
import traceback
import os
import gi
from gi.repository import Gdk
Copy link

Choose a reason for hiding this comment

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

No need to import.

from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
SCALE_Y = Gdk.Screen.height() / 900
Copy link

Choose a reason for hiding this comment

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

Use Pygame and not Gdk to get the initialised display surface dimensions. get_surface, get_width/height.

from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
SCALE_Y = Gdk.Screen.height() / 900
Copy link

Choose a reason for hiding this comment

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

Use Pygame not Gdk.

from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
SCALE_Y = Gdk.Screen.height() / 900
Copy link

Choose a reason for hiding this comment

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

Use Pygame not Gdk.

from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
SCALE_Y = Gdk.Screen.height() / 900
Copy link

Choose a reason for hiding this comment

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

Use Pygame not Gdk.

from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
SCALE_Y = Gdk.Screen.height() / 900
Copy link

Choose a reason for hiding this comment

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

SCALE_X and SCALE_Y are being calculated so many times; see if you can calculate them once and get them.

Copy link
Member

Choose a reason for hiding this comment

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

Can we import them from a utils.py ? This would still in principle import Gdk, but that would remove the clutter

Copy link

Choose a reason for hiding this comment

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

Yes, it could be in another module provided it was executed only once, not once per import. No, using Gdk would violate modular design. Gdk should only be used to obtain data for use with Gtk. Pygame has it's own API for getting the same data.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not too familiar with using pygame API to fix the display resolution issue...... I'll look it up and fix this. Thanks for mentioning it :)

from gi.repository import Gdk

SCALE_X = Gdk.Screen.width() / 1200
SCALE_Y = Gdk.Screen.height() / 900
Copy link

Choose a reason for hiding this comment

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

Use Pygame not Gdk.

@JuiP
Copy link
Member Author

JuiP commented Jun 9, 2020

3222606 still uses Gdk and not the pygame API.However, changes for the activity to support other display sizes are complete. From what I found, this is the only place where I can use get_surface, get_width/height most of the game folder files donot have a pygame canvas. And I don't see how I can use pygame in that case. Instead to avoid Gdk import in each file, I can create a file in game folder, calculate the scaling constants and import the constants. @quozl @srevinsaju @chimosky Is this the best approach?

@JuiP JuiP marked this pull request as ready for review June 9, 2020 16:41
@srevinsaju
Copy link
Member

I can create a file in game folder, calculate the scaling constants and import the constants.

This is equivalent to have them imported 😄

How about pygame.display.Info()

@JuiP
Copy link
Member Author

JuiP commented Jun 10, 2020

This is equivalent to have them imported smile

Yes, you are correct.

How about pygame.display.Info()

I couldn't find a way to have usable global variables SCALE_X and SCALE_Y. For files like : game/objects when you try to use pygame.display.get_surface() and then get_width, I get an error: NoneType object has no attribute get_width.

I also found out that the PyCut.py file isn't used anywhere. It is noticable from the import statements and to confirm, I commented out the file and run the activity. It's the same no errors.

@srevinsaju
Copy link
Member

I am not sure if I did understand what you wanted to convey

Python 3.8.3 (default, May 17 2020, 18:15:42) 
[GCC 10.1.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pygame
pygame 1.9.6
Hello from the pygame community. https://www.pygame.org/contribute.html
>>> pygame.init()
(6, 0)
>>> a = pygame.display.Info()
>>> a
<VideoInfo(hw = 0, wm = 1,video_mem = 0
         blit_hw = 0, blit_hw_CC = 0, blit_hw_A = 0,
         blit_sw = 0, blit_sw_CC = 0, blit_sw_A = 0,
         bitsize  = 32, bytesize = 4,
         masks =  (16711680, 65280, 255, 0),
         shifts = (16, 8, 0, 0),
         losses =  (0, 0, 0, 8),
         current_w = 1920, current_h = 1080
>

>>> a.current_w
1920
>>> a.current_h
1080
>>>
>>> pygame.display.init()
>>> b = pygame.display.set_mode()
>>> dir(b)
['__class__', '__copy__', '__delattr__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '_pixels_address', 'blit', 'blits', 'convert', 'convert_alpha', 'copy', 'fill', 'get_abs_offset', 'get_abs_parent', 'get_alpha', 'get_at', 'get_at_mapped', 'get_bitsize', 'get_bounding_rect', 'get_buffer', 'get_bytesize', 'get_clip', 'get_colorkey', 'get_flags', 'get_height', 'get_locked', 'get_locks', 'get_losses', 'get_masks', 'get_offset', 'get_palette', 'get_palette_at', 'get_parent', 'get_pitch', 'get_rect', 'get_shifts', 'get_size', 'get_view', 'get_width', 'lock', 'map_rgb', 'mustlock', 'scroll', 'set_alpha', 'set_at', 'set_clip', 'set_colorkey', 'set_masks', 'set_palette', 'set_palette_at', 'set_shifts', 'subsurface', 'unlock', 'unmap_rgb']
>>> b.get_width()
1920
>>> b.get_height()
1080

I guess you can easily calculate SCALE_X and SCALE_Y then

@chimosky
Copy link
Member

@JuiP said

3222606 still uses Gdk and not the pygame API.However, changes for the activity to support other display sizes are complete. From what I found, this is the only place where I can use get_surface, get_width/height most of the game folder files donot have a pygame canvas. And I don't see how I can use pygame in that case. Instead to avoid Gdk import in each file, I can create a file in game folder, calculate the scaling constants and import the constants. @quozl @srevinsaju @chimosky Is this the best approach?

I doubt https://github.com/sugarlabs/PyCut/blob/master/PyCut.py#L8 is the only place you can use get_surface, get_width and get_height as the file doesn't affect how the game runs in sugar, the file was created so the game can be run on the command line.

You can have a method in game.PyCutGame that returns what you need when called.

@JuiP said

I also found out that the PyCut.py file isn't used anywhere. It is noticable from the import statements and to confirm, I commented out the file and run the activity. It's the same no errors.

Yes, that should happen and I've explained why above.
Also, the activity instance is instantiated in PyCutActivity.py.

@JuiP
Copy link
Member Author

JuiP commented Jun 11, 2020

I tried doing what you suggested @chimosky. I successfully used pygame API for game/init.py file. However, while imports I keep getting errors. Have a look at this commit and the changes in game/objects/message_bubble.py 

I tried importing using from game import PyCutGame. It gives me an import error as in the image below. This maybe because, game init.py imports scenes, events, objects. the file game/objects/message_bubble.py by importing as  from game import PyCutGame is causing an import inside an import. 

PyCut_import_error

@quozl
Copy link

quozl commented Jun 22, 2020

Thanks. Reviewed.

  • I did comment on obtaining size from Gdk and using it in Pygame, but I'll put it differently; PyCut.py is best left independent of Gtk if possible, and so the size of the Pygame canvas should be set in response to a synthetic VIDEORESIZE event, so that any future changes to Sugargame or the toolkit will be properly reflected in an adjusted canvas size,
  • same with message_bubble.py; use the Pygame API not the Gdk API,
  • and the other files where you used Gdk instead of Pygame API.

@quozl
Copy link

quozl commented Jun 22, 2020

@srevinsaju is correct, you only need to call pygame.display.Info if you want to get the canvas dimensions quickly. Alternatively, preserve them when a resize occurs. A resize is expected when the app starts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants