Issue #8: no modules on root

[MerkleBoy]

Making modules more modular by refactoring them into a Module contract , which significantly streamlines things

A few considerations:

• for some reasons, each sub-repository in the monorepo are not using __Modules contracts to deploy said modules; instead, it seem MUD's command-line tool tries to deploy whatever he can find in that subfolder and just assumes it belongs in the same namespace, and deploys everything there (Tables, Systems, along with some World-level function signatures)

I feel like we should build a simple script that just uses the __Module implementation we have to deploy said module in a World, so that we have full control of the deployment script instead of relying on MUD's. There's also the issue with function signatures that are registered at World-level, which could lead to issues, on top of having to use autogenerated interfaces that doesn't match the genuine ones.

Also adding a library that packages all Smart-Object-Framework features into a useable "object" that you can instantiate to call any and all functions contained in that framework. There's an issue with the Solidity compiler, apparently there is no way to have an interface that contains function overloading, and use that interface as an input

==> in abi.encodeCall( Interface.foo, (a, b)) only works if there is only foo(uint a, uint b) defined in the interface.
If foo(uint a, uint b, string c) and foo(uint a, uint b) exists concurrently in Interface, the compiler will throw an error.
See related issue on GitHub : etherum/solidity/issues#12981

[0xxlegolas]

Could we also run pnpm run lint before pushing ?

[MerkleBoy]

Could we also run pnpm run lint before pushing ?

My bad, I thought I had my husky pre-commit hooks set up properly but it wasnt.

Fixed formatting in d402e83

[ccp-tezza]

@MerkleBoy

If foo(uint a, uint b, string c) and foo(uint a, uint b) exists concurrently in Interface, the compiler will throw an error.

I believe this is not quite correct.. the Solidity conflict only occurs if the method signatures are exactly the same. This behaviour holds for all contracts, not just libraries.

So, if there are different parameters in the two function calls, then their method signature will be different and no conflict will occur as you mentioned.

E.g.
having

foo(uint a, uint b)
foo(uint a, uint b)

will cause a conflict, but

foo(uint a, uint b)
foo(uint a, uint b, string c)

should not

[ccp-tezza]

@MerkleBoy

for some reasons, each sub-repository in the monorepo are not using __Modules contracts to deploy said modules; instead, it seem MUD's command-line tool tries to deploy whatever he can find in that subfolder and just assumes it belongs in the same namespace, and deploys everything there (Tables, Systems, along with some World-level function signatures)

As we move to having full modules in our monorepo (which we will treat as standalone npm packages at some point), I think each sub-repository should have its own set of individual test, config, and deployment scripts defined in its package.json. Then the monorepo scripts should call these individual scripts in order according to dependancy (Core Modules, Standalone Modules, Hook based Modules, Frontier Class Modules, etc...).

cc: @gunnigylfa

[ccp-tezza]

A few inline comments to get clarity and agreement on, but overall looking good!

One point of discussion:

I think there is a need for abstracting cross-module interaction. I'm not sure using the current lib pattern is the perfect solutions for this. It is a rather heavy thing for just exposing a namespace and interface.

That being said, I do not have an alternate solution ATM. So I think we should go with the current implementation for now, get feedback, and think about if there is a lighter way to accomplish this.

[MerkleBoy]

@MerkleBoy

If foo(uint a, uint b, string c) and foo(uint a, uint b) exists concurrently in Interface, the compiler will throw an error.

I believe this is not quite correct.. the Solidity conflict only occurs if the method signatures are exactly the same. This behaviour holds for all contracts, not just libraries.

So, if there are different parameters in the two function calls, then their method signature will be different and no conflict will occur as you mentioned.

E.g. having

foo(uint a, uint b)
foo(uint a, uint b)

will cause a conflict, but

foo(uint a, uint b)
foo(uint a, uint b, string c)

should not

unfortunately, that's an issue at the Compiler level; for some reason, Interface.foo returns function-type parameters and it is not currently able to support overloading. It's really weird, but it's a limitation of the compiler itself

In a perfect world it should work, yeah

[0xxlegolas]

@MerkleBoy

If foo(uint a, uint b, string c) and foo(uint a, uint b) exists concurrently in Interface, the compiler will throw an error.

I believe this is not quite correct.. the Solidity conflict only occurs if the method signatures are exactly the same. This behaviour holds for all contracts, not just libraries.

So, if there are different parameters in the two function calls, then their method signature will be different and no conflict will occur as you mentioned.

E.g. having

foo(uint a, uint b)
foo(uint a, uint b)

will cause a conflict, but

foo(uint a, uint b)
foo(uint a, uint b, string c)

should not

Yes I have used overloaded functions as well and it works fine.

[MerkleBoy]

@0xxlegolas
Yes I have used overloaded functions as well and it works fine.

Sorry, I didn't explain the issue properly.
Using overloaded functions works fine, yes; the issue is that, if you create a contract with overloaded functions,
and have an Interface contract for it, you can import that interface and use the Interface object to cycle through all of its function-type variables.
However... for some reason, the syntax Interface.foo will throw a compiler error if it is used in a Library, or through an interface, so that prevents us to pass it to abi.encodeCall(..)

import { Interface } from "./interface.sol"

Library MyLib {

  function foo() internal {
    abi.encodeCall(Interface.foo, new bytes(0));
    ~~~~~ compiler error: solidity(6675)
  }
}
This happens if Interface has an overloaded version of `foo()`, let me know if you can't replicate but it is a thing, infortunately
Below, a quote from Issue [solidity/13981](https://github.com/ethereum/solidity/issues/13981) :

> This is not limited to libraries. All functions in Solidity work this way. The first call works because there's only one overload that can take `uint256`. The second one is ambiguous because `uint128` is also implicitly convertible to `uint256` so both candidates are viable. We already have an issue about the compiler failing to explain that clearly: #9607.
> 
> The compiler could resolve the ambiguity by choosing the more fitting overload, it just does not do that yet. That's covered by #1256.
> 
> You should also be able to work around it by attaching only a specific overload for your type with the `using {L.f} for T` syntax but infortunately this is not yet implemented either: #3556.
> 
> Overall, this is a valid report and the behavior should be changed, but we already have issues covering all these problems so I'm closing this one as a duplicate.

[0xxlegolas]

@0xxlegolas
Yes I have used overloaded functions as well and it works fine.

Sorry, I didn't explain the issue properly. Using overloaded functions works fine, yes; the issue is that, if you create a contract with overloaded functions, and have an Interface contract for it, you can import that interface and use the Interface object to cycle through all of its function-type variables. However... for some reason, the syntax Interface.foo will throw a compiler error if it is used in a Library, or through an interface, so that prevents us to pass it to abi.encodeCall(..)

import { Interface } from "./interface.sol"

Library MyLib {

  function foo() internal {
    abi.encodeCall(Interface.foo, new bytes(0));
    ~~~~~ compiler error: solidity(6675)
  }
}
This happens if Interface has an overloaded version of `foo()`, let me know if you can't replicate but it is a thing, infortunately
Below, a quote from Issue [solidity/13981](https://github.com/ethereum/solidity/issues/13981) :

> This is not limited to libraries. All functions in Solidity work this way. The first call works because there's only one overload that can take `uint256`. The second one is ambiguous because `uint128` is also implicitly convertible to `uint256` so both candidates are viable. We already have an issue about the compiler failing to explain that clearly: #9607.
> 
> The compiler could resolve the ambiguity by choosing the more fitting overload, it just does not do that yet. That's covered by #1256.
> 
> You should also be able to work around it by attaching only a specific overload for your type with the `using {L.f} for T` syntax but infortunately this is not yet implemented either: #3556.
> 
> Overall, this is a valid report and the behavior should be changed, but we already have issues covering all these problems so I'm closing this one as a duplicate.

Now it makes sense, If we define overloaded functions in library then it does not recognise the functions unlike interfaces.
However I don't like the idea of having the interface definitions in the Library. (its kind of anti pattern).
For now we can keep it but add some documentation to the devs in future, that the interface definition will be separated from library and the entire module library (eg:SmartObjectLib) will be autogenerated.