chore: address comments

Author: zepfred

core/src/main/java/ai/timefold/solver/core/impl/AbstractFromConfigFactory.java

@@ -55,7 +55,9 @@ private EntityDescriptor<Solution_> getEntityDescriptorForClass(SolutionDescript
         EntityDescriptor<Solution_> entityDescriptor = solutionDescriptor.getEntityDescriptorStrict(entityClass);
         if (entityDescriptor == null) {
             throw new IllegalArgumentException(
-                    "The config (%s) has an entityClass (%s) that is not a known planning entity.\nCheck your solver configuration. If that class (%s) is not in the entityClassSet (%s), check your @%s implementation's annotated methods too."
+                    """
+                            The config (%s) has an entityClass (%s) that is not a known planning entity.
+                            Check your solver configuration. If that class (%s) is not in the entityClassSet (%s), check your @%s implementation's annotated methods too."""
                             .formatted(config, entityClass, entityClass.getSimpleName(), solutionDescriptor.getEntityClassSet(),
                                     PlanningSolution.class.getSimpleName()));
         }
@@ -98,9 +100,10 @@ protected GenuineVariableDescriptor<Solution_> getVariableDescriptorForName(Enti
         GenuineVariableDescriptor<Solution_> variableDescriptor = entityDescriptor.getGenuineVariableDescriptor(variableName);
         if (variableDescriptor == null) {
             throw new IllegalArgumentException(
-                    "The config (%s) has a variableName (%s) which is not a valid planning variable on entityClass (%s).\n%s"
-                            .formatted(config, variableName, entityDescriptor.getEntityClass(),
-                                    entityDescriptor.buildInvalidVariableNameExceptionMessage(variableName)));
+                    """
+                            The config (%s) has a variableName (%s) which is not a valid planning variable on entityClass (%s).
+                            %s""".formatted(config, variableName, entityDescriptor.getEntityClass(),
+                            entityDescriptor.buildInvalidVariableNameExceptionMessage(variableName)));
         }
         return variableDescriptor;
     }

core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java

@@ -57,15 +57,14 @@ public void solve(SolverScope<Solution_> solverScope) {
         phaseStarted(phaseScope);
 
         var solutionDescriptor = solverScope.getSolutionDescriptor();
-        var listVariableDescriptor = solutionDescriptor.getListVariableDescriptor();
-        var hasOnlyListVariable = listVariableDescriptor != null && !solutionDescriptor.hasBothBasicAndListVariables();
+        var hasListVariable = solutionDescriptor.hasListVariable();
         var maxStepCount = -1;
-        if (hasOnlyListVariable) {
+        if (hasListVariable) {
             // In case of list variable with support for unassigned values, the placer will iterate indefinitely.
             // (When it exhausts all values, it will start over from the beginning.)
             // To prevent that, we need to limit the number of steps to the number of unassigned values.
             var workingSolution = phaseScope.getWorkingSolution();
-            maxStepCount = listVariableDescriptor.countUnassigned(workingSolution);
+            maxStepCount = solutionDescriptor.getListVariableDescriptor().countUnassigned(workingSolution);
         }
 
         TerminationStatus earlyTerminationStatus = null;
@@ -102,7 +101,7 @@ public void solve(SolverScope<Solution_> solverScope) {
             doStep(stepScope);
             stepEnded(stepScope);
             phaseScope.setLastCompletedStepScope(stepScope);
-            if (hasOnlyListVariable && stepScope.getStepIndex() >= maxStepCount) {
+            if (hasListVariable && stepScope.getStepIndex() >= maxStepCount) {
                 earlyTerminationStatus = TerminationStatus.regular(phaseScope.getNextStepIndex());
                 break;
             } else if (phaseTermination.isPhaseTerminated(phaseScope)) {

core/src/main/java/ai/timefold/solver/core/impl/constructionheuristic/placer/QueuedMultiplePlacer.java

@@ -1,6 +1,7 @@
 package ai.timefold.solver.core.impl.constructionheuristic.placer;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -47,13 +48,26 @@ private class MultipleQueuedPlacingIterator extends UpcomingSelectionIterator<Pl
             implements PhaseLifecycleListener<Solution_> {
 
         private final List<EntityPlacer<Solution_>> queuedPlacerList;
-        private List<Iterator<Placement<Solution_>>> placementIteratorList;
-        private List<Iterator<Move<Solution_>>> moveIteratorList;
+        private Iterator<Move<Solution_>>[] moveIterators;
+        private Iterator<Placement<Solution_>>[] placementIterators;
         private Move<Solution_>[] previousMove;
         private Move<Solution_> cachedMove = null;
 
         private MultipleQueuedPlacingIterator(List<EntityPlacer<Solution_>> queuedPlacerList) {
-            this.queuedPlacerList = new ArrayList<>(queuedPlacerList);
+            // We expect only the QueuedValuePlacer and a QueuedEntityPlacer
+            var assertSize = queuedPlacerList.size() == 2;
+            var assertQueuedValuePlacer =
+                    queuedPlacerList.stream().anyMatch(QueuedValuePlacer.class::isInstance);
+            var assertQueuedEntityPlacer =
+                    queuedPlacerList.stream().anyMatch(QueuedEntityPlacer.class::isInstance);
+            if (!assertSize || !assertQueuedValuePlacer || !assertQueuedEntityPlacer) {
+                throw new IllegalArgumentException(
+                        "Impossible state: the queued placer list must consist exclusively of a QueuedValuePlacer and a QueuedEntityPlacer.");
+            }
+            this.queuedPlacerList = new ArrayList<>();
+            // We make sure that the QueuedEntityPlacer is added first
+            this.queuedPlacerList.addAll(queuedPlacerList.stream().filter(QueuedValuePlacer.class::isInstance).toList());
+            this.queuedPlacerList.addAll(queuedPlacerList.stream().filter(QueuedEntityPlacer.class::isInstance).toList());
             reset();
         }
 
@@ -62,27 +76,28 @@ private MultipleQueuedPlacingIterator(List<EntityPlacer<Solution_>> queuedPlacer
          * but it uses placer iterators instead.
          */
         private Move<Solution_> nextMove() {
-            if (cachedMove == null) {
-                var childSize = moveIteratorList.size();
-                int index;
-                Move<Solution_>[] move = new Move[childSize];
-                if (previousMove == null) {
-                    index = -1;
-                } else {
-                    index = consumeNextMove(move, previousMove);
-                    if (index == -1) {
-                        // No more moves
-                        return null;
-                    }
-                }
-                var recreated = recreateNextIterators(index, move);
-                if (!recreated) {
-                    // We stop if one of the placement iterators has no next placement
+            if (cachedMove != null) {
+                return cachedMove;
+            }
+            var childSize = moveIterators.length;
+            int index;
+            Move<Solution_>[] move = new Move[childSize];
+            if (previousMove == null) {
+                index = -1;
+            } else {
+                index = consumeNextMove(move, previousMove);
+                if (index == -1) {
+                    // No more moves
                     return null;
                 }
-                previousMove = move;
-                cachedMove = CompositeMove.buildMove(move);
             }
+            var updatedMove = updateNextIterators(index, move);
+            if (updatedMove == null) {
+                // We stop if one of the placement iterators has no next placement
+                return null;
+            }
+            previousMove = updatedMove;
+            cachedMove = CompositeMove.buildMove(updatedMove);
             return cachedMove;
         }
 
@@ -98,10 +113,21 @@ private int consumeNextMove(Move<?>[] move, Move<Solution_>[] previousMove) {
             var index = move.length - 1;
             // Look for the first iterator that still has available moves to generate
             while (index >= 0) {
-                var moveIterator = moveIteratorList.get(index);
+                var moveIterator = moveIterators[index];
                 if (moveIterator.hasNext()) {
                     break;
                 }
+                // Check if there are more placements available in the QueuedEntityPlacer
+                if (index == 1) {
+                    var placementIterator = placementIterators[index];
+                    if (placementIterator.hasNext()) {
+                        moveIterators[index] = placementIterator.next().iterator();
+                        continue;
+                    } else {
+                        // Reset the iterator in case the previous placerIterator still has more placements
+                        placementIterators[index] = queuedPlacerList.get(index).iterator();
+                    }
+                }
                 index--;
             }
             if (index < 0) {
@@ -110,43 +136,53 @@ private int consumeNextMove(Move<?>[] move, Move<Solution_>[] previousMove) {
             // Copy the previous move until the next one generated
             System.arraycopy(previousMove, 0, move, 0, index);
             // Generate and set the new move
-            move[index] = moveIteratorList.get(index).next();
+            move[index] = moveIterators[index].next();
             return index;
         }
 
         /**
-         * Recreate all move iterators starting from #lastValidIteratorIndex.
+         * Update the move list and recreate all move iterators starting from #lastValidIteratorIndex.
          *
          * @param lastValidIteratorIndex the index of the last iterator that generated a valid move
          * @param move the move array to be loaded
          */
-        private boolean recreateNextIterators(int lastValidIteratorIndex, Move<?>[] move) {
-            for (int i = lastValidIteratorIndex + 1; i < move.length; i++) {
-                var placementIterator = queuedPlacerList.get(i).iterator();
-                placementIteratorList.set(i, placementIterator);
+        private Move<Solution_>[] updateNextIterators(int lastValidIteratorIndex, Move<?>[] move) {
+            var childSize = moveIterators.length;
+            var updatedMove = new Move[childSize];
+            System.arraycopy(move, 0, updatedMove, 0, childSize);
+            for (int i = lastValidIteratorIndex + 1; i < childSize; i++) {
+                var placementIterator = placementIterators[i];
                 Move<Solution_> next;
                 if (!placementIterator.hasNext()) {
-                    return false;
+                    return null;
                 } else {
                     var moveIterator = placementIterator.next().iterator();
-                    moveIteratorList.set(i, moveIterator);
+                    moveIterators[i] = moveIterator;
                     next = moveIterator.next();
                 }
-                move[i] = next;
+                updatedMove[i] = next;
             }
-            return true;
+            return updatedMove;
         }
 
         private void clearCache() {
             this.cachedMove = null;
         }
 
+        @SuppressWarnings("unchecked")
         private void reset() {
-            placementIteratorList = new ArrayList<>(queuedPlacerList.size());
-            moveIteratorList = new ArrayList<>(queuedPlacerList.size());
-            for (EntityPlacer<Solution_> placements : queuedPlacerList) {
-                moveIteratorList.add(null);
-                placementIteratorList.add(null);
+            if (moveIterators == null) {
+                moveIterators = new Iterator[queuedPlacerList.size()];
+                Arrays.fill(moveIterators, null);
+                placementIterators = new Iterator[queuedPlacerList.size()];
+                for (var i = 0; i < queuedPlacerList.size(); i++) {
+                    var placement = queuedPlacerList.get(i);
+                    placementIterators[i] = placement.iterator();
+                }
+            } else {
+                Arrays.fill(moveIterators, null);
+                // We need to reset of the QueuedEntityPlacer or there will be no more moves for the basic variables
+                placementIterators[1] = queuedPlacerList.get(1).iterator();
             }
             previousMove = null;
         }

core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java

@@ -707,6 +707,16 @@ public boolean hasAnyGenuineBasicVariables() {
                 .anyMatch(descriptor -> !descriptor.isListVariable());
     }
 
+    public boolean hasAnyGenuineChainedVariables() {
+        if (!isGenuine()) {
+            return false;
+        }
+        return getDeclaredGenuineVariableDescriptors().stream()
+                .filter(descriptor -> descriptor instanceof BasicVariableDescriptor<Solution_>)
+                .map(descriptor -> (BasicVariableDescriptor<Solution_>) descriptor)
+                .anyMatch(BasicVariableDescriptor::isChained);
+    }
+
     public boolean hasAnyGenuineListVariables() {
         if (!isGenuine()) {
             return false;

core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/SolutionDescriptor.java

@@ -727,13 +727,9 @@ private void validateListVariableDescriptors() {
 
         var listVariableDescriptor = listVariableDescriptorList.get(0);
         var listVariableEntityDescriptor = listVariableDescriptor.getEntityDescriptor();
-        var hasChained = listVariableEntityDescriptor.getGenuineVariableDescriptorList().stream()
-                .anyMatch(
-                        variableDescriptor -> variableDescriptor instanceof BasicVariableDescriptor<Solution_> basicVariableDescriptor
-                                && basicVariableDescriptor.isChained());
         // We will not support chained and list variables at the same entity,
         // and the validation can be removed once we discontinue support for chained variables.
-        if (hasChained) {
+        if (hasChainedVariable()) {
             var basicVariableDescriptorList = new ArrayList<>(listVariableEntityDescriptor.getGenuineVariableDescriptorList());
             basicVariableDescriptorList.remove(listVariableDescriptor);
             throw new UnsupportedOperationException(
@@ -883,8 +879,20 @@ public PlanningSolutionMetaModel<Solution_> getMetaModel() {
         return planningSolutionMetaModel;
     }
 
+    public boolean hasBasicVariable() {
+        return getGenuineEntityDescriptors().stream().anyMatch(EntityDescriptor::hasAnyGenuineBasicVariables);
+    }
+
+    public boolean hasChainedVariable() {
+        return getGenuineEntityDescriptors().stream().anyMatch(EntityDescriptor::hasAnyGenuineChainedVariables);
+    }
+
+    public boolean hasListVariable() {
+        return getListVariableDescriptor() != null;
+    }
+
     public boolean hasBothBasicAndListVariables() {
-        return getMetaModel().hasBothBasicAndListVariables();
+        return hasBasicVariable() && hasListVariable();
     }
 
     /**

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/anchor/AnchorShadowVariableDescriptor.java

@@ -92,4 +92,9 @@ public Iterable<VariableListenerWithSources<Solution_>> buildVariableListeners(S
                 sourceVariableDescriptor).toCollection();
     }
 
+    @Override
+    public boolean isListVariableSource() {
+        return false;
+    }
+
 }

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/cascade/CascadingUpdateShadowVariableDescriptor.java

@@ -165,6 +165,11 @@ public Iterable<VariableListenerWithSources<Solution_>> buildVariableListeners(S
         throw new UnsupportedOperationException("Cascade update element generates no listeners.");
     }
 
+    @Override
+    public boolean isListVariableSource() {
+        return false;
+    }
+
     private record ShadowVariableTarget<Solution_>(EntityDescriptor<Solution_> entityDescriptor,
             MemberAccessor variableMemberAccessor) {
 

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/custom/CustomShadowVariableDescriptor.java

@@ -148,4 +148,9 @@ public Iterable<VariableListenerWithSources<Solution_>> buildVariableListeners(S
             return new VariableListenerWithSources<>(variableListener, classListEntry.getValue());
         }).collect(Collectors.toList());
     }
+
+    @Override
+    public boolean isListVariableSource() {
+        return false;
+    }
 }

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/custom/LegacyCustomShadowVariableDescriptor.java

@@ -231,4 +231,9 @@ public Iterable<VariableListenerWithSources<Solution_>> buildVariableListeners(S
         return new VariableListenerWithSources<>(variableListener, sourceVariableDescriptorList).toCollection();
     }
 
+    @Override
+    public boolean isListVariableSource() {
+        return false;
+    }
+
 }

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/custom/PiggybackShadowVariableDescriptor.java

@@ -110,4 +110,9 @@ public Iterable<VariableListenerWithSources<Solution_>> buildVariableListeners(S
         throw new UnsupportedOperationException("The piggybackShadowVariableDescriptor (" + this
                 + ") cannot build a variable listener.");
     }
+
+    @Override
+    public boolean isListVariableSource() {
+        return false;
+    }
 }