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

WIP Set tile id in map node #967

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft

WIP Set tile id in map node #967

wants to merge 27 commits into from

Conversation

SimplyLiz
Copy link
Contributor

@SimplyLiz SimplyLiz commented May 29, 2022

wip...

Remaining tasks:

  • Clicking on water crashes game (empty tile ID) - initial m_originCoordinate value wasn't part of the move constructor
  • Buildings can be placed multiple times
  • Check multi tiles if placement is allowed
  • Serialize origin coords

@SimplyLiz SimplyLiz self-assigned this May 29, 2022
Copy link
Member

@Mograbi Mograbi left a comment

Choose a reason for hiding this comment

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

@SimplyLiz why removed the const ?
it's mainly meant for checking and it makes sense that it will be const

@SimplyLiz
Copy link
Contributor Author

@SimplyLiz why removed the const ?
it's mainly meant for checking and it makes sense that it will be const

Because I need to use getMapNode instead of direct access and this must be a reference and I didn’t want to introduce a new function that does absolutely the same but is const

@Mograbi
Copy link
Member

Mograbi commented Jun 8, 2022

@SimplyLiz why removed the const ?
it's mainly meant for checking and it makes sense that it will be const

Because I need to use getMapNode instead of direct access and this must be a reference and I didn’t want to introduce a new function that does absolutely the same but is const

if it's exactly the same why not use const then ? didn't understand

@SimplyLiz
Copy link
Contributor Author

@SimplyLiz why removed the const ?
it's mainly meant for checking and it makes sense that it will be const

Because I need to use getMapNode instead of direct access and this must be a reference and I didn’t want to introduce a new function that does absolutely the same but is const

if it's exactly the same why not use const then ? didn't understand

Because in a lot of cases we’ll need to Manipulate the mapnode objects. And in some we don’t which would need an exact duplicate of the getNode function but being const

Mograbi
Mograbi previously approved these changes Jun 13, 2022
@sonarcloud
Copy link

sonarcloud bot commented Jun 17, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 77 Code Smells

40.7% 40.7% Coverage
0.0% 0.0% Duplication

@iaGuoZhi
Copy link
Member

Hi! @SimplyLiz , Can I work on thie pr now? I want to fix this problem: #892.

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

Successfully merging this pull request may close these issues.

3 participants