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

Slight changes item_set to item_map in SelectAction function #81

Merged
merged 19 commits into from
Nov 20, 2023

Conversation

Gaya1858
Copy link
Contributor

@Gaya1858 Gaya1858 commented Nov 1, 2023

No description provided.

Gaya1858 and others added 9 commits October 17, 2023 14:50
…eate VectoreMaze done and also added an asset.
…an area of mRenderRange around the player agent.
…nfrastructure built. menu now hold unique_ptr to buttons. maininterface has a font member used by reference by menu,button,textbox,messageboard
… works without layer. Also added menu button that a player can choose normal or enlarge mode.
@Gaya1858
Copy link
Contributor Author

We have created enlarge view functionality for the larger worlds. It works as expected.

Copy link
Contributor

@FergusonAJ FergusonAJ left a comment

Choose a reason for hiding this comment

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

I've added various comments that are probably worth looking at but I won't halt the PR for.

The one thing I request is to remove the changes to CMakeLists.txt and move those lines to the relevant .cmake files in source (more details in that comment)

CMakeLists.txt Outdated Show resolved Hide resolved
source/Interfaces/Button.cpp Show resolved Hide resolved
source/Interfaces/MainInterface.cpp Outdated Show resolved Hide resolved
mWindow.display();
}
/**
* @brief Creates a 9x23 window of the symbol grid centered around the player's position.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm missing where 9 and 23 are coming from. Are they related to COL and ROW? If so, we might want to make those names a little more clear (e.g., NUM_ROWS or something)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, found them in the header. I do think the name could be a little more clear.

source/Interfaces/Menu.cpp Show resolved Hide resolved
source/Interfaces/MessageBoard.h Outdated Show resolved Hide resolved
@Gaya1858
Copy link
Contributor Author

We have incorporated your suggestions

Copy link
Contributor

@FergusonAJ FergusonAJ left a comment

Choose a reason for hiding this comment

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

Requested changes have been made. Thanks!

@Gaya1858 Gaya1858 merged commit e28aa2a into MSU-CSE491:main Nov 20, 2023
1 check passed
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.

4 participants