-
Notifications
You must be signed in to change notification settings - Fork 19
(#484) Exploration objectives #531
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
base: main
Are you sure you want to change the base?
Conversation
f229af1 to
eb2f283
Compare
| public RiftSpace getSpaceAt(Vec3i pos) { | ||
| return getSpaceAt(pos.getX(), pos.getY(), pos.getZ()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RiftLayout interface has exact same method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it doesn't. It has getChunkSpace, which is more of a getOrCreateSpaceAt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice that the name is different. But this new overload still seems to be unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, it ended up unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different types of RiftGeneratable should probably have their own identifier type since each is identified with different information. Builtins usually only need ResourceLocation, but payload templates need ResourceLocation and palette index.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having multiple types adds complexity for no value, particularly because it makes it harder to work with the identifier. Also not clear there is a use case for future variations. I'll at least make index an OptionalInt and change the string representation to not have an index for the built in cases.
src/main/java/com/wanderersoftherift/wotr/world/level/ConnectedRoomIterator.java
Outdated
Show resolved
Hide resolved
|
|
||
| @SubscribeEvent | ||
| public static void onDiedInRift(RiftEvent.PlayerDied event) { | ||
| // TODO: Rather than sending an event, have a registry of GoalTracking providers? Pros, cons |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how much improvement would that change get, but the current state is kinda weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave it alone for now then. Would be easier to change after #536
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, I did a rework. GoalTrackers are now looked up using (registerable) lookup functions, and then they can provide a stream of GoalStates of specific types that can be worked with directly rather than passing around update functions.
src/main/java/com/wanderersoftherift/wotr/core/goal/GoalTracker.java
Outdated
Show resolved
Hide resolved
|
|
||
| import java.util.List; | ||
|
|
||
| public record MultiplyNumberProvider(List<NumberProvider> values) implements NumberProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm starting to think that maybe we should make a registry for some common math functions and whenever there is datapacking involving math we just make one "math" type that has function property. Or give it expression string to parse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could change the rift parameters to lean more on the NumberProvider system. Ignoring the name, LootContext is a pretty flexible mechanism for providing context to number generation.
|
I've played with these changes, and they do work as intended. And we did catch a few bugs with it, but that should all be working alright now. |
refactor: merged rift package into core.rift refactor: moved goals into their own package
feat: replace kill objective with goal based version
…atterns of jigsaw with a room distance of entrance refactor: cleanup and comments
feat: room rift count number provider feat: additional math operator number providers feat: RiftMapData for tracking rooms discovered/visited refactor: use ICancellableEvent for GoalEvent.Update
refactor: replaced use of LootParams with LootContext for goal and reward generation refactor: removed kill objective
refactor: moved goal widgets from quest to goal package
feat: objective block structures feat: goal based objectives now fall back on the no objective their goals lack targets feat: exit portals spawn throughout the rift at low probability
fix: corrected room rift counting
feat: recipe update for new objectives
refactor: reduced duplicate code refactor: improved some naming
refactor: Simplfied hasNext in ConnectedRoomIterator refactor: Improved RiftGeneratableId
refactor: Moved objective data into its own level attachment refactor: Reworked goal update handling refactor: various cleanup to goal handling
fix: Check right level for objective when leaving rift
… and added some error handling
29509a9 to
b5a0cbe
Compare
Closes #484
Closes #461
Closes #536
Overview
Introduces three new objectives centered around exploration - closing anomalies, entering rooms and activating objective blocks. Additionally adds a chance for exit portals to spawn in the rift.
Testing Instructions
wotr riftKey objectivecommand.Framework Changes
GoalEvent.Updateevent, so they can be subscribed to by both quests and objectives (or other systems in the future). By reusing the goal system it means whenever a new goal type is implemented it can be used by both - with this update for example it is also possible to add quests to close anomalies, visit rooms and activate objective blocks.RiftMapEvent.RoomFirstVisitedEventis broadcast from here.rift_parameternumber provider to get the value from a rift parameter. Additionally added number providers for obtaining rift room and rift structure counts, and math operator providers for combining different loot parameters (currently multiply, sum, ceil, max, and min). This all enables the use of rift information in goals as part of GoalBasedObjectives.Smaller Changes
AnomalyEvent.ClosedGoalEvent.Updatefor the ActivateObjectiveGoal.wotr rift roomInfocommand is nowwotr rift room info, and awotr rift room anomalyCountcommand has been added.ReplaceSingleJigsawJigsawListProcessor for having a chance to replace a single jigsaw pool in a generatable.FastWeightedList.codecWithSingleAlternativemethod for providing a codec for a FastWeightedList with support for a single value being provided instead.RiftSpaceCorridorto obtain the location of the connected space.ConnectedRoomIteratorfor iterating rooms connected to a room, outwards to a desired depth.Bug Fixes
RiftConfig#withCustomDatawhen attempting to override an existing entry.Refactors
core.goal, out fromcore.quest.GoalStateandGoalTrackerinterfaces and simplifying UI element linkage to goals from different contexts.RiftDifficultyEventsandRiftEventstoRiftDifficultyEventHandlerandRiftEventHandler. I would suggest that aXEventHandlernaming pattern for the classes subscribing to events is clearer than theXEventspattern, as it separates them better from theXEventnaming for classes providing event types.Implementation Notes