Randomized map generator #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… files - The CardBase applies all effects when played - The application of effect is done given the card data (parser TODO) - This application will be done to all targets in the list - This calls the apply_effect which is done to individual targets
…o one target at once
It worked nicely. This card probably won't stay as it is, that would be OP But it's nice to test that the system is working
Closed
Turtyo
reviewed
Dec 21, 2023
|
i'm also realizing the room doesn't have an EventBase element, but you will need to wait for #29 to be closed to be able to reference that properly (which will also give you access to the list of possible events) |
I'm not sure if I forgot something that was mentionned during the PR or not
* Create Event Parent and Event Child Classes * Issue 20: Update Event Children with scene string instantiation, Create Events Classification script to hold enums for different events and added it to AutoLoader * Issue 20: Add comment for test function * Issue-20: Rename Event classes, remove EventsClassification enum * Issue-20: Update EventBase to have array of different types of events. Update init() as well. * Issue-20: Update EventBase Array holding all Events, Add Unit Test for EventBase
create_map() now generates to reflect the map shape reference (https://github.com/Saplings-Projects/1M_sub/issues/6#issue-2027403379)(https://github.com/Saplings-Projects/1M_sub/pull/30#issuecomment-1868931059) changed floors_width to reflect basic math
added static typing where there was not any
Turtyo
requested changes
Dec 31, 2023
There was a problem hiding this comment.
Thank you for implementing the idea I gave earlier @multi-arm !
I think it needs some changes.
This would probably be a good thing to test in the map tests also (generate a small map and compare it to a reference we have, to be sure the padding an the rooms all are correctly set)
|
I'm also seeing you added some typing for functions in the latest commit, could you take a look at these comments i left at the beginning about some missing types ? |
|
Sanity checking this, I think we need to sync up |
Static typing changes Added create_map return type changed map generation (optimization) (MapBase.rooms holds arrays of rooms, one for each floor)
create_map now has configurable width for testing changed test for a small floor (reference)
fixed unintended test changes
|
If it's just for the Contributing.md, I don't think it's really needed to sync main in map (it's not a code dependency for example) |
Turtyo
requested changes
Jan 2, 2024
Tysterman74
requested changes
Jan 3, 2024
…s-Projects#37) * Cut all of the unnecessary code in CardBase * Changes to the card resource to make sure they work * Removed unnecessarily _damage_entirt function * Removed cards that don't do anything
changed create_map(_layout) > create_map(floors_width) added room padding to both before and after rooms are generated changed tests to use a fixed list
* Added a WIP buttom that shows a new scene * The button now shows a new scene with a return button to go back to the game. The game cannot be played while the scene is present. * The button now shows a new scene with a return button to go back to the game. The game cannot be played while the scene is present. * added Typing * Added CardScrollUI to see the inside of a CardPile * Added CardScrollUI to see the inside of a CardPile * Changed event to input_event and added its type * Updated the the min size of the CardUI * Fixed the aftermath of merging * Added constant for the card. * Deleted a function that is irrelevant. * Moved the ShowCardPile into CardPileUISetter * Deck doesn't need to have a card count. * resized ScrollContainer to show 5 columns and 2 rows * Added a button to show cards inside the deck * Fixed the code that modifies the card and adds it to the container. * Whitespace be gone * Changed the settings of GridContainer so cards first show in the center * Print statement begone * added the "pile" after deck so it is consistent to the others pile * Added a test for cardscrollui * Fixed the test so it gets the proper gridcontainer * Changed _input function to use a key constant instead of a string. * Changed the settings of GridContainer so cards first show in the center part 2 * Addressed review feedback from Ty * Addressed review feedback from Turtyo * Changed the test so it compares Cardbases * added a comment to the input function. * potential fix to errors in test * amendment for the last commit * Fixed a function, fixed the test?, and cards not appearing in the UI. * Fixed the testing scene after the merge * whitespace be gone --------- Co-authored-by: Turtyo <49365680+Turtyo@users.noreply.github.com>
Turtyo
reviewed
Jan 9, 2024
Turtyo
reviewed
Jan 9, 2024
Tysterman74
previously approved these changes
Jan 10, 2024
changed create_map(layout) to create_map(map_floors_width) changed insert in create_map() to append changed test error message formatting added override _to_string() in RoomBase, this will have more added in the future
Turtyo
reviewed
Jan 13, 2024
changed assert_not_null(test_map.rooms) to assert_not_null(_room) in map test
Turtyo
approved these changes
Jan 17, 2024
|
I pulled in local to test, it loads and all the tests are passing. We don't have any visual for now but the code seems good, and this is a solid basis imo. Thank you @multi-arm ! |
Tysterman74
approved these changes
Jan 17, 2024
Tysterman74
merged commit b4d84ad
into
Saplings-Projects:map-implementation
1 check passed
Closed
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added base classes for map, floor and room
Added manager script to manage said base classes
Related issues
Issue 10 (#10), Issue 6 (#6)
List of changes
Created MapBase, FloorBase and RoomBase classes
Created MapManager script
Created test_map
Tests
Added one test for generation. More tests will be added in the future
Additional notes
Trying this again, everything should be fixed now hopefully but I'll make this a draft too just in case.