lavenderdotpet / LibreQuake

A freesoftware quake one remake with art under the BSD license
https://discord.gg/nsr6DTF6RX
Other
418 stars 34 forks source link

BUILD: Fail map compile on missing textures #174

Closed pnahratow closed 1 month ago

pnahratow commented 1 month ago

Description of Changes

This change makes a map compile fail if there are textures missing.

Visual Sample

I have tested this on the faulty commit in !173 and it worked

Checklist


pnahratow commented 1 month ago

Someone needs to fix WARNING: unable to find texture hp25_side in lq1/maps/src/brushmodels/b_bh100.map

@lavenderdotpet you maybe?

pnahratow commented 1 month ago

I think fail_on_regex should be a list containing multiple regex expressions so it's easier to add new failure cases in the future. Other than that, looks good.

I thought about that as well and started to implement that but then I realized I can catch most of that with a regex already. Like

r"(One String|Some completely different string)"

or different types of warnings

r"WARNING: (unable to find texture|something different)"

So I dropped that. Do you agree?

Zungrysoft commented 1 month ago

@pnahratow

I thought about that as well and started to implement that but then I realized I can catch most of that with a regex already.

I think that gets pretty messy if you have to catch a lot of different cases. And there are definitely other warnings we may like to add in the future. It would be more readable to have each case be a separate string on a separate line.

pnahratow commented 1 month ago

@Zungrysoft I have implemented it using a list. Thanks for the review.

lavenderdotpet commented 1 month ago

Someone needs to fix WARNING: unable to find texture hp25_side in lq1/maps/src/brushmodels/b_bh100.map

@lavenderdotpet you maybe?

ill take a look prob an internal unseen face

edit: yeah image

ill put a dummy texture in for it

lavenderdotpet commented 1 month ago

@pnahratow add this to the lq_health_ammo folder

hp25_side

MotoLegacy commented 1 month ago

@pnahratow add this to the lq_health_ammo folder

hp25_side

Could you make this a separate PR instead? So we can keep them focused.

lavenderdotpet commented 1 month ago

@pnahratow add this to the lq_health_ammo folder

hp25_side

Could you make this a separate PR instead? So we can keep them focused.

image kinda has to be this one and its a single file

lavenderdotpet commented 1 month ago

@pnahratow add this to the lq_health_ammo folder

hp25_side

Could you make this a separate PR instead? So we can keep them focused.

image kinda has to be this one and its a single file

edit: also like its the only thing preventing this being merged other then a 2nd aprovel

MotoLegacy commented 1 month ago

We can rerun the build here once the file is merged without issue, and because it's critical status it can be merged right away.

I won't intervene further, but keep in mind this treads the line between focused v unfocused and probably should not land when we start guideline enforcement

lavenderdotpet commented 1 month ago

We can rerun the build here once the file is merged without issue, and because it's critical status it can be merged right away.

I won't intervene further, but keep in mind this treads the line between focused v unfocused and probably should not land when we start guideline enforcement

heard will approve then

lavenderdotpet commented 1 month ago

this is now being done

@ pnahratow add this to the lq_health_ammo folder

hp25_side

@pnahratow ignore this ↑ this is now being done over here ↓ https://github.com/lavenderdotpet/LibreQuake/pull/176