Conversation
|
Important Review skippedToo many files! This PR contains 250 files, which is 100 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (50)
📒 Files selected for processing (250)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
While reviewing the files on GitHub, I noticed several issues (e.g., incorrect spacing and unwanted symbols). Additionally, some values were modified since my last update that were not intended to be changed (e.g., HP). |
|
Very nice writeup on the changes, good job! |
Lowercase file paths Revert whitespace changes Revert changes to copyright symbol in unit scripts
lL1l1
left a comment
lL1l1
left a comment
There was a problem hiding this comment.
Aeon factories reviewed:
Aeon T1->T2 land factory upgrade detaches the left arm which looks weird because its physically impossible and the factory becomes asymmetrical during the upgrade. Not sure about the solution, it is a difficult problem.
Aeon T1 navy fac water effects and hitbox do not match the mesh.
Do you know if Scrolling = true was used on the aeon factor lod 0 meshes?
|
@IL1I1 thank you very much for your testing. Aeon T1 Land Factory Upgrade Unfortunately, while creating the animation, I couldn’t come up with a better solution than the current one. Previously, both roof elements kept their positions during the upgrade process. Since the tower positions change in the T2 HQ and Support models, I considered this approach to be the most reasonable way to visually represent the upgrade. I may revise the entire Aeon Land Factory model again. I’ve received feedback from multiple sources expressing the wish for a model that more closely resembles the original design from before the changes. Aeon Naval Factory – Hitbox / Effects I have not implemented effects of this kind in the project so far. However, I am still aware of the hitbox issue. If I decide to adjust the hitbox, I would also review other hitboxes as part of that process, since some of them do not fully match the updated model. DescriptionThe following hitboxes need adjustment, as they currently do not properly match their respective models: Affected Units Cybran Land Factory Seraphim Air Factory Aeon Naval Factory Expected Behavior Hitboxes accurately match the visible model dimensions. Consistent hitbox structure for units sharing the same base model. No regression of previously identified bugs. To Do Increase width of the Cybran Land Factory hitbox Apply T3 hitbox to all Seraphim Air Factory models Reduce Aeon Naval Factory hitbox size Perform regression testing for the known issue Below is a short overview along with images of the currently used hitboxes in the game. Lines marked in yellow indicate a deviation from the other tech levels of the same 
|
|
I'm currently reviewing and correcting all the blueprints and scripts. After that, I'll work on the hitboxes. Since I haven't quite understood Git yet, I would be grateful if someone could review this work afterwards. |
All Lua files in the factory models have been reviewed and revised. Changes: Removed unnecessary whitespace Adjusted path naming to match the common convention (fixed upper/lower case) Added the AverageDensity = parameter to support factories, as it is also present in all HQs Corrected incorrect health values Special Note: Since each factory variant is based on the T1 variant, the DDS files (Albedo, NormalTS, SpecTeam, LOD1, etc.) are referenced from there. Therefore, the files in the higher tech tiers are no longer required. If this commit does not remove those files, the core team may handle their removal after confirmation through testing by other contributors.
Here are the minor hitbox adjustments for various units. UEF Land T1 & T2 Increased SizeZ from 4.4 to 4.8 Air T3 Increased SizeZ from 3.5 to 4.2 AEON Air T1 Increased SizeX from 3 to 3.5 Increased SizeZ from 3 to 3.5 Naval T1 SizeZ = 13 This value remains unchanged because modifying it triggers a bug that shifts the build footprint (see first comment). The issue was narrowed down to the Physics section under: SkirtOffset SkirtSize It has now also been observed that the hitbox itself affects this issue. Since the build footprint can influence the faction behavior, this value remains unchanged for now until a better solution is found. Cybran Land T1, T2, T3 Increased SizeX from 2.9 to 4.3 Increased SizeZ from 4.2 to 4.3 Naval T3 SizeX = 4 This value was intended to be adjusted. However, since the model’s root bone is heavily aligned with the build cranes, a large empty space in front of the factory is included. Therefore, this value was not changed. Seraphim Air T1, T2, T3 Set SizeX to 3.5 Set SizeY to 2.7 Set SizeZ to 3.5 Since the model changes very little between tech levels, the hitbox has been standardized and averaged. Previously, the dimensions were partially too small or too tall. From my side, the project is now complete unless further issues are discovered.
|
Proposal: Adjustment of the Aeon Naval Hitbox The proposed adjustment should be carefully reviewed and evaluated. Purpose of the change The hitbox will be aligned more accurately with the existing model, ensuring better visual and technical consistency with the actual factory size. 
Potential impact The hitbox will more closely match the model. However, when multiple factories are placed in a row, their build position may shift after upgrading to the next tech level. The factories remain fully functional. There is a possibility of model overlap if units were placed before upgrading to the next tech level. 
Detailed Changes Aeon Naval T1, T2, T3 T1 SkirtOffsetZ: -1 → -2 SizeZ: 13 → 10.5 T2 SkirtOffsetZ: -1 → -2 SizeZ: 13 → 12 T3 SkirtOffsetZ: -1 → 0 SizeZ: 13 → 14 The current state of the project only presents the possibility that the hitbox of the T1 version is slightly too large. However, all other errors have been corrected. |
+ AI TargetBones +Hitbox SizeY =1
Target bone adjustment - B08 to B29 - Supplement B01
Minor adjustment to the size of the water effect, as this has partially disappeared under the new model.
BlackYps
left a comment
BlackYps
left a comment
There was a problem hiding this comment.
I left some comments, other than that it looks good.
units/UAB0101/UAB0101_unit.bp
Outdated
| AlbedoName = "/units/uab0101/uab0101_albedo.dds", | ||
| NormalsName = "/units/uab0101/uab0101_normalsts.dds", | ||
| SpecularName = "/units/uab0101/uab0101_specteam.dds", |
There was a problem hiding this comment.
Aren't these just default values here? I don't think they are needed when the texture is actually from the same unit. The higher tier factories do need explicit texture paths, because they reference textures from a different unit.
Same thing in other T1 units.
There was a problem hiding this comment.
You're probably right. Since I'd only worked with mods before, I overlooked the issue with the T1 models.
The current FAF only refers to
AlbedoName = "ueb0101_lod1_albedo.dds",
SpecularName = "ueb0101_lod1_specteam.dds",
| UAB0102 = ClassUnit(AAirFactoryUnit) {} | ||
|
|
||
| TypeClass = UAB0102 | ||
| TypeClass = UAB0102 No newline at end of file |
There was a problem hiding this comment.
You have this change in all the script files. Please get rid of it. It's probably because of a setting in your editor. See: https://gist.github.com/olivertappin/19a95236d8b69f1c9eadfc799a9ab7fc
There was a problem hiding this comment.
Thank you for the explanation. I use Note++ and didn't know how the symbol came about or what it meant.
I'll make the change tomorrow.
| UISelection = Sound { Bank = 'Interface', Cue = 'Aeon_Select_Factory', LodCutoff = 'UnitMove_LodCutoff' }, | ||
| }, | ||
| AverageDensity = 6400, | ||
| AverageDensity = 6600, |
There was a problem hiding this comment.
Isn't this value irrelevant on buildings?
There was a problem hiding this comment.
Unfortunately, I don't know. However, since this value is included in the blueprints of the standard game, I've added it to all factories accordingly.
There was a problem hiding this comment.
I'm not a fan of the new textures of the UEF land factory. They look too clean and the normals are inverted in the sense that ridges seem to stick out. I'd just remove these textures for now.
We can always think about a texture rework at a later date in a separate pull request.
There was a problem hiding this comment.
That's perfectly fine with me. I will remove the file first; it is still available in my mod on FAF if necessary.
little explanation about this:
I took this on because Jip suggested it and Baltazar made his work available to me.
Mesh path descriptions have been reverted in all T1 models, as mentioned by BlackYps, since this path is not required.
3 participants
### Redesign of all HQ and Support Factories 2.0
As described in the forum post (https://forum.faforever.com/topic/5790/redesign-of-all-hq-and-support-factories), the first step towards redesigning the factory models was started some time ago. Since this work was unfortunately never completed, I have picked up the project again and continued it.
Overview of Changes
All models were recreated and adjusted based on the FAF templates, enabling upgrades to HQ and Support factories
All models now have their own build animations
LOD1 models were largely adjusted to match the new LOD0 models
### Specific Changes
Build Area of Naval Factories
The required build area for all naval factories has been adjusted.
This change is based on the observation that when an existing factory was upgraded to a higher tech level, the build footprint shifted once the mesh expanded.
This adjustment prevents this issue/effect from occurring (see forum post
https://forum.faforever.com/topic/5790/redesign-of-all-hq-and-support-factories/26).
Visual Adjustments to Aeon T3 Naval Ship Construction
As described in forum post (19), the Mercury Pool visuals were displayed disproportionately large compared to the ships.
These visuals have now been adjusted to better match the ship sizes (https://forum.faforever.com/topic/5790/redesign-of-all-hq-and-support-factories/63).
Added Death Animation for Seraphim Naval Factories
While creating the death animations for the Seraphim faction, it was discovered that the animations were not being played.
This issue was fixed by adding the PlayAnimation(...) function to the relevant scripts.
See: (https://forum.faforever.com/topic/5790/redesign-of-all-hq-and-support-factories/90)
Adjustment of TargetBones for Land and Air Factories
During testing, I noticed that some units were unable to damage certain factories as intended. Since no changes have been made to the hitboxes so far, this issue is likely caused by the reorientation of various bones in the updated models.
To verify this assumption, several tests were conducted with modified configurations. As a result, the UEF, Aeon, and Cybran factory models now use the following configuration:
AI = {
TargetBones = {
"Attachpoint",
},
},
I would suggest revising the hitboxes of the land and air factories. Since this issue was only briefly mentioned in the forum post, here are a few images to illustrate the point more clearly.