I think we could add this operation in BasicLighthingModel removing this.environment, and extends PhongLightingModel from BasicLightingModel

Okay, I rewritten the code like that!

One thing that can be optimized though (ideally with a different PR) is that NodeMaterial.setupLights() should only add an instance of EnvironmentNode for PBR materials. All other materials which do not support PMREM are incompatible with that type of node. Imo, it seems more clean if this node isn't created for that materials in the first place.

I made a small revision. The idea of ​​the LightingModel parameters would be equivalent to the defines that enable the specific modules of the light model, normally boolean, since we could access the material in the setup process.

Still, I have to do another review, ideally we should have a node like EnvironmentNode to handle this.

Awesome, that looks much cleaner now!

Besides, I love these kind of reviews. They are ideal to learn the details of the new system^^.

ideally we should have a node like EnvironmentNode to handle this.

Indeed. It would be nice if NodeMaterial could somehow setup a different type of EnvironmentNode depending on the type of material. That would automatically solve #28798 (comment).

Still, I have to do another review, ideally we should have a node like EnvironmentNode to handle this.

I done this part! :)

Great! Let's merge the current state. We can add more revisions in other PRs.