fix: layout and pagination items overlap

inventory-framework-api/src/main/java/me/devnatan/inventoryframework/component/Pagination.java

@@ -1,8 +1,6 @@
 package me.devnatan.inventoryframework.component;
 
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 import me.devnatan.inventoryframework.state.StateValue;
 import me.devnatan.inventoryframework.state.StateValueHost;
@@ -221,27 +219,20 @@ public interface Pagination extends ComponentComposition, StateValue {
      *
      * @param index The page index.
      * @param pageSize Number of elements that each page can have.
-     * @param pagesCount Pre-calculated total number of pages available (set zero if not available).
+     * @param pagesCount Total number of pages available (zero if not available) [exclusive].
      * @param src   The source to split.
      * @return All elements in a page.
      * @throws IndexOutOfBoundsException If the specified index is {@code < 0} or
      *                                   exceeds the pages count.
      */
     static List<?> splitSourceForPage(int index, int pageSize, int pagesCount, List<?> src) {
         if (src.isEmpty()) return Collections.emptyList();
-
-        if (src.size() <= pageSize) return new ArrayList<>(src);
-        if (index < 0 || (pagesCount > 0 && index > pagesCount))
+        if (index < 0 || (pagesCount > 0 && index >= pagesCount))
             throw new IndexOutOfBoundsException(String.format(
                     "Page index must be between the range of 0 and %d. Given: %d", pagesCount - 1, index));
 
-        final List<Object> contents = new LinkedList<>();
-        final int base = index * pageSize;
-        int until = base + pageSize;
-        if (until > src.size()) until = src.size();
-
-        for (int i = base; i < until; i++) contents.add(src.get(i));
-
-        return contents;
+        int fromIndex = index * pageSize;
+        int toIndex = Math.min(fromIndex + pageSize, src.size());
+        return src.subList(fromIndex, toIndex);
     }
 }

inventory-framework-core/src/main/java/me/devnatan/inventoryframework/component/PaginationImpl.java

@@ -5,6 +5,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
@@ -244,31 +245,60 @@ private void addComponentsForUnconstrainedPagination(IFRenderContext context, Li
      * @param pageContents Elements of the current page.
      */
     private void addComponentsForLayeredPagination(IFRenderContext context, List<?> pageContents) {
-        final LayoutSlot targetLayoutSlot = getLayoutSlotForCurrentTarget(context);
-        final int elementsLen = pageContents.size();
-        debug("[Pagination] Elements count: %d elements", elementsLen);
-        debug("[Pagination] Iterating over '%c' layout target", targetLayoutSlot.getCharacter());
-
-        int iterationIndex = 0;
-        for (final int position : targetLayoutSlot.getPositions()) {
-            final Object value = pageContents.get(iterationIndex++);
+        final LayoutSlot layoutSlot = getLayoutSlotForCurrentTarget(context);
+        debug("[Pagination] Is layout slot defined by the user? %b", layoutSlot.isDefinedByTheUser());
+
+        final int contentSize = pageContents.size();
+        debug("[Pagination] Elements count: %d elements", contentSize);
+        debug("[Pagination] Iterating over '%c' layout target", layoutSlot.getCharacter());
+
+        int index = 0;
+        for (final int layoutPosition : layoutSlot.getPositions()) {
+            if (index >= contentSize) {
+                // A layout slot defined by the user mean the user probably want to add a placeholder
+                // item to fill empty slots that couldn't be reached by the pagination. Happens when
+                // using pagination and #layoutSlot with the same character as the pagination.
+                if (layoutSlot.isDefinedByTheUser()) {
+                    final ComponentFactory componentFactory =
+                            layoutSlot.getFactory().apply(index);
+
+                    if (componentFactory instanceof ItemComponentBuilder) {
+                        final ItemComponentBuilder<?, ?> itemBuilder = (ItemComponentBuilder<?, ?>) componentFactory;
+
+                        // In normal scenarios item position are set in LayoutRenderInterceptor
+                        // but since it uses the same layout target as a Pagination component,
+                        // This is the same behavior as `PaginationStateBuilder#elementFactory`.
+                        itemBuilder.withSlot(layoutPosition);
+
+                        // Marking as "externally managed" prevents other components from modifying
+                        // this item accidentally.
+                        itemBuilder.withExternallyManaged(true);
+                    }
+
+                    final Component component = componentFactory.create();
+
+                    debug(() -> "  @ placeholder %d (index %d) = %s", layoutPosition, index, component.toString());
+                    getInternalComponents().add(component);
+                    index++;
+                    continue;
+                }
+
+                break; // EOF
+            }
 
             try {
-                final ComponentFactory factory = elementFactory.create(this, iterationIndex, position, value);
+                final Object paginatedValue = pageContents.get(index);
+                final ComponentFactory factory = elementFactory.create(this, index, layoutPosition, paginatedValue);
                 final Component component = factory.create();
 
-                debug(
-                        () -> "  @ added %d (index %d) = %s",
-                        position,
-                        iterationIndex,
-                        component.getClass().getSimpleName());
+                debug(() -> "  @ added %d (index %d) = %s", layoutPosition, index, component.toString());
                 getInternalComponents().add(component);
             } catch (final Exception exception) {
-                debug(() -> "  @ failed to add %d (index %d) = %s", position, iterationIndex, exception.getMessage());
+                debug(() -> "  @ failed to add %d (index %d) = %s", layoutPosition, index, exception.getMessage());
                 exception.printStackTrace();
             }
 
-            if (iterationIndex == elementsLen) break;
+            index++;
         }
     }
 
@@ -298,11 +328,12 @@ private LayoutSlot getLayoutSlotForCurrentTarget(IFRenderContext context) {
 
         final Optional<LayoutSlot> layoutSlotOptional = context.getLayoutSlots().stream()
                 .filter(layoutSlot -> layoutSlot.getCharacter() == getLayoutTarget())
-                .findFirst();
+                .max(Comparator.comparing(LayoutSlot::isDefinedByTheUser));
 
-        if (!layoutSlotOptional.isPresent())
+        if (!layoutSlotOptional.isPresent()) {
             // TODO more detailed error message
             throw new IllegalArgumentException(String.format("Layout slot target not found: %c", getLayoutTarget()));
+        }
 
         return (currentLayoutSlot = layoutSlotOptional.get());
     }
@@ -458,7 +489,7 @@ public void clear(@NotNull IFContext context) {
     }
 
     @Override
-    public @UnmodifiableView Set<State<?>> getWatchingStates() {
+    public @UnmodifiableView Set<me.devnatan.inventoryframework.state.State<?>> getWatchingStates() {
         return Collections.emptySet();
     }
 

inventory-framework-core/src/main/java/me/devnatan/inventoryframework/context/AbstractIFContext.java

@@ -121,21 +121,27 @@ private IFSlotRenderContext createSlotRenderContext(@NotNull Component component
 
     @Override
     public void renderComponent(@NotNull Component component) {
-        if (!component.shouldRender(this)) {
-            component.setVisible(false);
+        if (component.shouldRender(this)) {
+            component.render(createSlotRenderContext(component, false));
+            return;
+        }
 
-            final Optional<Component> overlapOptional = getOverlappingComponentToRender(this, component);
-            if (overlapOptional.isPresent()) {
-                Component overlap = overlapOptional.get();
-                renderComponent(overlap);
+        component.setVisible(false);
 
-                if (overlap.isVisible()) return;
-            }
+        final Optional<Component> overlapOptional = getOverlappingComponentToRender(this, component);
+        if (overlapOptional.isPresent()) {
+            Component overlap = overlapOptional.get();
+            renderComponent(overlap);
 
-            component.clear(this);
-            return;
+            if (overlap.isVisible()) {
+                IFDebug.debug(
+                        "Component was not rendered due to overlapping component (component = %s, overlap = %s)",
+                        component, overlap);
+                return;
+            }
         }
-        component.render(createSlotRenderContext(component, false));
+
+        component.clear(this);
     }
 
     private Optional<Component> getOverlappingComponentToRender(ComponentContainer container, Component subject) {

inventory-framework-core/src/main/java/me/devnatan/inventoryframework/pipeline/AvailableSlotInterceptor.java

@@ -22,9 +22,12 @@ public void intercept(PipelineContext<VirtualView> pipeline, VirtualView subject
         final IFRenderContext context = (IFRenderContext) subject;
         if (context.getAvailableSlotFactories() == null) return;
 
-        final List<ComponentFactory> slotComponents = context.getConfig().getLayout() == null
-                ? resolveFromInitialSlot(context)
-                : resolveFromLayoutSlot(context);
+        final List<ComponentFactory> slotComponents;
+        if (context.getConfig().getLayout() == null) {
+            slotComponents = resolveFromInitialSlot(context);
+        } else {
+            slotComponents = resolveFromLayoutSlot(context);
+        }
 
         slotComponents.forEach(componentFactory -> context.addComponent(componentFactory.create()));
     }

inventory-framework-core/src/main/java/me/devnatan/inventoryframework/pipeline/LayoutRenderInterceptor.java

@@ -1,11 +1,15 @@
 package me.devnatan.inventoryframework.pipeline;
 
+import java.util.AbstractMap;
+import java.util.Map;
 import java.util.function.IntFunction;
+import java.util.stream.Collectors;
 import me.devnatan.inventoryframework.InventoryFrameworkException;
 import me.devnatan.inventoryframework.VirtualView;
 import me.devnatan.inventoryframework.component.Component;
 import me.devnatan.inventoryframework.component.ComponentFactory;
 import me.devnatan.inventoryframework.component.ItemComponentBuilder;
+import me.devnatan.inventoryframework.component.Pagination;
 import me.devnatan.inventoryframework.context.IFRenderContext;
 import me.devnatan.inventoryframework.internal.LayoutSlot;
 
@@ -16,6 +20,14 @@ public void intercept(PipelineContext<VirtualView> pipeline, VirtualView subject
         if (!(subject instanceof IFRenderContext)) return;
 
         final IFRenderContext renderContext = (IFRenderContext) subject;
+        final Map<Character, Component> paginationComponents = renderContext.getComponents().stream()
+                .filter(component -> component instanceof Pagination)
+                .map(component -> {
+                    final Pagination pagination = (Pagination) component;
+                    return new AbstractMap.SimpleImmutableEntry<>(pagination.getLayoutTarget(), pagination);
+                })
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
         for (final LayoutSlot layoutSlot : renderContext.getLayoutSlots()) {
             final IntFunction<ComponentFactory> factory = layoutSlot.getFactory();
             if (factory == null) {
@@ -25,6 +37,13 @@ public void intercept(PipelineContext<VirtualView> pipeline, VirtualView subject
                 continue;
             }
 
+            // Pagination component are rendered AFTER `availableSlot`/`layoutSlot`.
+            // Once a layout slot that uses the same layout target as the pagination component one
+            // is detected, pagination component takes care of them and uses them as fallback.
+            if (paginationComponents.containsKey(layoutSlot.getCharacter())) {
+                continue;
+            }
+
             int iterationIndex = 0;
             for (final int slot : layoutSlot.getPositions()) {
                 final ComponentFactory componentFactory = factory.apply(iterationIndex++);

inventory-framework-platform/src/main/java/me/devnatan/inventoryframework/context/PlatformRenderContext.java

@@ -1,6 +1,5 @@
 package me.devnatan.inventoryframework.context;
 
-import static java.lang.String.format;
 import static me.devnatan.inventoryframework.utils.SlotConverter.convertSlot;
 
 import java.util.ArrayList;
@@ -168,8 +167,6 @@ public final void availableSlot(@NotNull BiConsumer<Integer, T> factory) {
      * @return An item builder to configure the item.
      */
     public final @NotNull T layoutSlot(char character) {
-        requireNonReservedLayoutCharacter(character);
-
         // TODO More detailed exception message
         final LayoutSlot layoutSlot = getLayoutSlots().stream()
                 .filter(value -> value.getCharacter() == character)
@@ -191,8 +188,6 @@ public final void availableSlot(@NotNull BiConsumer<Integer, T> factory) {
      * @param character The layout character target.
      */
     public final void layoutSlot(char character, @NotNull BiConsumer<Integer, T> factory) {
-        requireNonReservedLayoutCharacter(character);
-
         // TODO More detailed exception message
         final LayoutSlot layoutSlot = getLayoutSlots().stream()
                 .filter(value -> value.getCharacter() == character)
@@ -355,18 +350,5 @@ private void checkAlignedContainerTypeForSlotAssignment() {
                     "Non-aligned container type %s cannot use row-column slots, use absolute %s instead",
                     getContainer().getType().getIdentifier(), "#slot(n)"));
     }
-
-    /**
-     * Checks if the character is a reserved layout character.
-     *
-     * @param character The character to be checked.
-     * @throws IllegalArgumentException If the given character is a reserved layout character.
-     */
-    private void requireNonReservedLayoutCharacter(char character) {
-        if (character == LayoutSlot.FILLED_RESERVED_CHAR)
-            throw new IllegalArgumentException(format(
-                    "The '%c' character cannot be used because it is only available for backwards compatibility. Please use another character.",
-                    character));
-    }
     // endregion
 }