feat: expose settlement events #31
Conversation
|
Neat! Throwing some reviewers at this, and pinging @Vizaxo who weirdly for some reason still doesn't auto-complete for reviews etc here. |
| @@ -180,7 +180,7 @@ public void postBegin() { | |||
| return Optional.empty(); | |||
| } | |||
|
|
|||
| public Optional<GenericBuildingComponent> getRandomBuildingOfZoneForCulture(String zone, Rect2i shape, CultureComponent cultureComponent) { | |||
| public GenericBuildingComponent getRandomBuildingOfZoneForCulture(String zone, Rect2i shape, CultureComponent cultureComponent) { | |||
There was a problem hiding this comment.
Why did you change this interface? I personally think that Optional conveys the intended interface way better than the Java's typical null pattern. I still find it counter-intuitive that a method public X getFoo() might return "nothing" as part of its interface.
| @@ -38,8 +38,7 @@ | |||
| import org.terasology.dynamicCities.buildings.components.ProductionChestComponent; | |||
| import org.terasology.dynamicCities.buildings.components.SettlementRefComponent; | |||
| import org.terasology.dynamicCities.buildings.events.OnSpawnDynamicStructureEvent; | |||
| import org.terasology.dynamicCities.construction.events.BuildingEntitySpawnedEvent; | |||
| import org.terasology.dynamicCities.construction.events.SpawnStructureBufferedEvent; | |||
| import org.terasology.dynamicCities.construction.events.*; | |||
There was a problem hiding this comment.
Please avoid star imports (in IntelliJ you have to set the number of classes for which start import is triggered to some very high number)
| @@ -99,6 +100,7 @@ | |||
| import org.terasology.world.block.entity.placement.PlaceBlocks; | |||
| import org.terasology.world.generation.Border3D; | |||
| import org.terasology.world.generation.facets.SurfaceHeightFacet; | |||
| import sun.misc.Request; | |||
There was a problem hiding this comment.
Is this import intended? I general, you should avoid the sun packages if possible.
| import org.terasology.dynamicCities.settlements.events.CheckBuildingSpawnPreconditionsEvent; | ||
| import org.terasology.dynamicCities.settlements.events.SettlementGrowthEvent; | ||
| import org.terasology.dynamicCities.settlements.events.SettlementRegisterEvent; | ||
| import org.terasology.dynamicCities.settlements.events.*; |
There was a problem hiding this comment.
Revert the start import.
| && checkBuildArea) { | ||
| CheckSiteSuitabilityEvent checkSiteSuitabilityEvent = new CheckSiteSuitabilityEvent(); | ||
| siteRegion.send(checkSiteSuitabilityEvent); | ||
| if (checkSiteSuitabilityEvent.getResult() == SettlementFilterResult.SUITABLE) { |
There was a problem hiding this comment.
You probably want to use a switch statement here (also helps to spot missed cases, etc.).
switch(checkSiteSuitabilityEvent.getResult()) {
case SettlementFilterResult.SUITABLE:
...;
break;
case SettlementFilterResult.UNSUITABLE:
case SettlementFilterResult.UNKOWN:
...;
break;
}| */ | ||
| public class CheckBuildingForParcelEvent implements ConsumableEvent { | ||
| public DynParcel dynParcel; | ||
| public GenericBuildingComponent building; |
There was a problem hiding this comment.
As mentioned above, I'd like to have this as Optional
|
No (technical) reason, just a matter of personal taste. I've put it back in and removed the star imports, etc. |
|
Re-bump just to see if we should merge yet :-) Pinging @skaldarnar @Vizaxo @msteiger @CptCrispyCrunchy |
src/main/java/org/terasology/dynamicCities/construction/Construction.java
Outdated
Show resolved
Hide resolved
skaldarnar
added
the
Type: Improvement
skaldarnar
changed the title
Exposes some events for site selection, building selection, and block rasterization to allow customization by client code. There are probably still more events to expose but it seems good to merge incrementally. Part of #29
As a side note, I think
RasterTargetshould be used in place ofworldProviderorblockBufferSystemdirectly. I didn't want to make that change since it may be included in the major changes already underway. If that's implemented the newSetandBufferblock events and handlers can be removed.