Skip to content

Conversation

@MangoS9
Copy link

@MangoS9 MangoS9 commented Sep 7, 2023

Description

Added in support for vector array and static array for center_camera_on() method for C++ as calculating multiple sprite locations can be tedious.

void center_camera_on(vector<sprite> s, double offset_x, double offset_y);

parameters:

  • vector<sprite> s
  • double offset_x
  • double offset_y
void center_camera_on(sprite s[],int size , double offset_x, double offset_y);

parameters:

  • sprite s[]
  • int size
  • double offset_x
  • double offset_y
    Both supports vector_2d data sets too:
	//static arr
	void center_camera_on(sprite s[],int size , const vector_2d &offset);
	//vector arr
	void center_camera_on(vector<sprite> s, const vector_2d &offset)

Video demo:

2023-09-07.23-12-04.mp4

Type of change

  • Overloading the method in camera.cpp and camera.h file
  • Modified the camera test to check the functionality

How Has This Been Tested?

  • Tested with Windows 10
  • Test on single variable, vector array and static array

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Requested a review from seniors on the Pull Request

Copy link
Member

@macite macite left a comment

Choose a reason for hiding this comment

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

This needs work, and should have been reviewed before submitting here. Reviews should critically check basic things like indentation.

void center_camera_on(sprite s, double offset_x, double offset_y)
{
point_2d center = sprite_position(s);
// void center_camera_on(sprite s, double offset_x, double offset_y)
Copy link
Member

Choose a reason for hiding this comment

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

These comments should not remain in the code. Only include comments that add value.

double sc_x{0};
double sc_y{0};

for (const auto& ele : s) {
Copy link
Member

Choose a reason for hiding this comment

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

don't use auto

* @param offset_y An additional offset added to the camera, allowing you to
* position the sprites offset from the center of the screen.
*/
void center_camera_on(sprite s[],int size , double offset_x, double offset_y);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the translator supports arrays - have you tested rebuilding the language translations. See the tools/scripts/deploy.sh script.

* @param offset The amount to offset the camera, allowing you to position
* the sprites away from the center of the screen.
*/
void center_camera_on(sprite s[], int size, const vector_2d &offset);
Copy link
Member

Choose a reason for hiding this comment

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

As above

* @attribute method center_on
*/
void center_camera_on(sprite s, const vector_2d &offset);
/**
Copy link
Member

Choose a reason for hiding this comment

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

Very sloppy!

Match indentation.


load_bitmap("ufo", "ufo.png");
sprite s = create_sprite("ufo");
sprite s1 = create_sprite("ufo");
Copy link
Member

Choose a reason for hiding this comment

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

Why not just in the vector and then access them there by index when needed.

Copy link
Member

Choose a reason for hiding this comment

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

Then use a loop to run repeated instructions

}

update_sprite(s);
update_sprite(s1);
Copy link
Member

Choose a reason for hiding this comment

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

Loop

draw_triangle(COLOR_AQUA, screen_width() / 2, 0, 0, screen_height(), screen_width(), screen_height());

draw_sprite(s);
draw_sprite(s1);
Copy link
Member

Choose a reason for hiding this comment

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

Loop

center_camera_on(s, offset.x, offset.y);
}

void center_camera_on(sprite s[],int size , const vector_2d &offset)
Copy link
Member

Choose a reason for hiding this comment

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

Sloppy!

@macite macite marked this pull request as draft September 9, 2023 02:15
@MangoS9
Copy link
Author

MangoS9 commented Sep 9, 2023

I will make the changes once I got back to my computer 🙌

@MangoS9
Copy link
Author

MangoS9 commented Sep 9, 2023

strangely enough, I didnt notice the indentation error when I'm working on it. Usually VS code should have warned me about it 🙁

@MangoS9
Copy link
Author

MangoS9 commented Sep 9, 2023

Update: It seems like the indentation of "camera.h "on my local computer looks fine, I do not know why this happens in github but I will get it fixed real soon and have my senior to get it checked up

@MangoS9
Copy link
Author

MangoS9 commented Sep 9, 2023

Update:

  • fixed the indentation (Hopefully)
  • Reformat the test_camera code into a more readable manner
  • Changed "auto" type to "sprite"
  • Removed all the unnecessary comment in the camera.h
  • Run the deploy.sh with no issue found
    image

Will get someone to review in case it is wrong again

@MangoS9 MangoS9 marked this pull request as ready for review September 10, 2023 04:18
@MangoS9 MangoS9 requested a review from macite September 11, 2023 22:14
@MangoS9 MangoS9 closed this by deleting the head repository Sep 13, 2023
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