Line 216 repeatedly reads a state variable when it doesn't need to. Instead s.installationTypes.length could be assigned to a local variable outside the loop and the local variable could be used in the loop.
Line 218 InstallationType memory installationType = s.installationTypes[_installationTypes[i]]; is very gas inefficient because it reads all the state variables from the InstallationType struct from contract storage but not all the variables are used in the function, and it does this within a loop. I know I said to assign to a memory variable like this. I was wrong.
Line 242 is updating a state variable in a loop. It could be updated one time after the loop. Before the loop a local variable could be assigned the value of s.nextCraftId and be used and updated within the loop. The local variable could also be read at line 239 instead of the s.nextCraftId state variable.
Various gas optimizations without resorting to assembly can be made. Basic information about gas optimization is in this article: https://eip2535diamonds.substack.com/p/smart-contract-gas-optimization-with The data from that article could be applied to all code.
For gas optimization I only reviewed the function below and give my observations:
https://github.com/aavegotchi/aavegotchi-realm-diamond/blob/17278b72edfc1c4915ac3bb8fc7f8c8d2ebfb974/contracts/InstallationDiamond/facets/InstallationFacet.sol#L212-L246
Line 216 repeatedly reads a state variable when it doesn't need to. Instead
s.installationTypes.length
could be assigned to a local variable outside the loop and the local variable could be used in the loop.Line 218
InstallationType memory installationType = s.installationTypes[_installationTypes[i]];
is very gas inefficient because it reads all the state variables from the InstallationType struct from contract storage but not all the variables are used in the function, and it does this within a loop. I know I said to assign to a memory variable like this. I was wrong.Line 242 is updating a state variable in a loop. It could be updated one time after the loop. Before the loop a local variable could be assigned the value of
s.nextCraftId
and be used and updated within the loop. The local variable could also be read at line 239 instead of thes.nextCraftId
state variable.